diff --git a/assets/js/Components/FixIssuesPage.js b/assets/js/Components/FixIssuesPage.js index ee9d39a5e..83bb75079 100644 --- a/assets/js/Components/FixIssuesPage.js +++ b/assets/js/Components/FixIssuesPage.js @@ -463,7 +463,10 @@ export default function FixIssuesPage({ setFilteredIssues(tempFilteredContent) setGroupedList(groupList(tempFilteredContent)) - setActiveIssue(holdoverActiveIssue) + if(holdoverActiveIssue) { + setActiveIssue(holdoverActiveIssue) + } + }, [report]) @@ -702,7 +705,7 @@ export default function FixIssuesPage({ return tempDoc.body.innerHTML } - const handleIssueSave = (issue, markAsReviewed = false) => { + const handleIssueSave = async (issue, markAsReviewed = false) => { if(!activeContentItem || !activeContentItem?.body || !issue) { return @@ -734,83 +737,73 @@ export default function FixIssuesPage({ let fullPageDoc = new DOMParser().parseFromString(fullPageHtml, 'text/html') let newElement = Html.findElementWithError(fullPageDoc, issue?.newHtml) let newXpath = Html.findXpathFromElement(newElement) - if(newXpath) { - issue.xpath = newXpath - } - else { - issue.xpath = "" - } - activeContentItem.body = fullPageHtml - + issue.xpath = newXpath || '' + // Save the updated issue using the LMS API let api = new Api(settings) try { - api.saveIssue(issue, fullPageHtml, markAsReviewed) - .then((responseStr) => { - // Check for HTTP errors before parsing JSON - if (!responseStr.ok) { - processServerError(responseStr) - updateActiveSessionIssue(issue.id, settings.ISSUE_STATE.ERROR) - removeItemFromBeingScanned(issue.contentItemId) - return null - } - return responseStr.json() - }) - .then((response) => { + const saveResponse = await api.saveIssue(issue, fullPageHtml, markAsReviewed) + if(!saveResponse.ok){ + processServerError(saveResponse) + updateActiveSessionIssue(issue.id, settings.ISSUE_STATE.ERROR) + removeItemFromBeingScanned(issue.contentItemId) + throw Error('Save returned invalid server response.') + } - // If the save falied, show the relevant error message - if (response.data.failed) { - updateActiveSessionIssue(issue.id, settings.ISSUE_STATE.ERROR) - removeItemFromBeingScanned(issue.contentItemId) - response.messages.forEach((msg) => addMessage(msg)) - - if (Array.isArray(response.data.issues)) { - response.data.issues.forEach((issue) => { - addMessage({ - severity: 'error', - message: t(`form.error.${issue.ruleId}`) - }) - }) - } + const saveResponseJson = await saveResponse.json() - if (Array.isArray(response.data.errors)) { - response.data.errors.forEach((error) => { - addMessage({ - severity: 'error', - message: error - }) + if(saveResponseJson?.errors && saveResponseJson.errors.length > 0) { + updateActiveSessionIssue(issue.id, settings.ISSUE_STATE.ERROR) + removeItemFromBeingScanned(issue.contentItemId) + saveResponseJson.messages.forEach((msg) => addMessage(msg)) + + if (Array.isArray(saveResponseJson.errors)) { + saveResponseJson.errors.forEach((error) => { + addMessage({ + severity: 'error', + message: t(error) }) - } + }) } - else { - - // If the save was successful, show the success message - response.messages.forEach((msg) => addMessage(msg)) - - if (response.data.issue) { - // Update the report object by rescanning the content - const newIssue = Object.assign({}, issue, response.data.issue) - // const formattedData = formatIssueData(newIssue) - // setActiveIssue(formattedData) - updateActiveSessionIssue(newIssue.id, settings.ISSUE_STATE.SAVED) - - api.scanContent(newIssue.contentItemId) - .then((responseStr) => responseStr.json()) - .then((res) => { - const tempReport = Object.assign({}, res?.data) - processNewReport(tempReport) - removeItemFromBeingScanned(newIssue.contentItemId) - }) - } - else { - // setActiveIssue(formatIssueData(issue)) - updateActiveSessionIssue(issue.id, settings.ISSUE_STATE.SAVED) - removeItemFromBeingScanned(issue.contentItemId) + throw Error('Save failed.') + } + + // Successful save! + saveResponseJson.messages.forEach((msg) => addMessage(msg)) + if(issue.contentItemId === activeContentItem.id) { + activeContentItem.body = fullPageHtml + } + + // If there isn't a new issue created, we're done. + if(!saveResponseJson?.data?.issue) { + updateActiveSessionIssue(issue.id, settings.ISSUE_STATE.SAVED) + removeItemFromBeingScanned(issue.contentItemId) + return + } + + if(saveResponseJson?.data?.issue) { + // Update the report object by rescanning the content + const newIssue = Object.assign({}, issue, saveResponseJson.data.issue) + updateActiveSessionIssue(newIssue.id, settings.ISSUE_STATE.SAVED) + + const scanResponse = await api.scanContent(newIssue.contentItemId) + + if(scanResponse.ok) { + const scanResponseJson = await scanResponse.json() + if(scanResponseJson.data) { + const tempReport = Object.assign({}, scanResponseJson.data) + processNewReport(tempReport) + removeItemFromBeingScanned(newIssue.contentItemId) + return } } - }) + } + + updateActiveSessionIssue(issue.id, settings.ISSUE_STATE.SAVED) + removeItemFromBeingScanned(issue.contentItemId) + } catch (error) { - console.error(error) + console.warn(error) updateActiveSessionIssue(issue.id, settings.ISSUE_STATE.ERROR) } } diff --git a/assets/js/Components/Forms/AltText.js b/assets/js/Components/Forms/AltText.js index c83145947..5e1befd80 100644 --- a/assets/js/Components/Forms/AltText.js +++ b/assets/js/Components/Forms/AltText.js @@ -54,11 +54,18 @@ export default function AltText ({ let element = Html.toElement(html) if (isDecorative) { - element = Html.setAttribute(element, "role", "presentation") + + // elements may have special roles, like "img", and we don't want to overwrite or remove those. + // elements with type="image" also need to keep their role. + if(issue.scanRuleId !== 'canvas_content_described' && issue.scanRuleId !== 'imagebutton_alt_exists') { + element = Html.setAttribute(element, "role", "presentation") + } element = Html.setAttribute(element, 'alt', '') element = Html.setAttribute(element, 'title', '') } else { - element = Html.removeAttribute(element, "role") + if(issue.scanRuleId !== 'canvas_content_described' && issue.scanRuleId !== 'imagebutton_alt_exists') { + element = Html.removeAttribute(element, "role") + } element = Html.setAttribute(element, "alt", textInputValue) element = Html.setAttribute(element, "title", textInputValue) } diff --git a/assets/js/Components/Forms/SensoryMisuseForm.js b/assets/js/Components/Forms/SensoryMisuseForm.js index 6305e9446..a232dbf8c 100644 --- a/assets/js/Components/Forms/SensoryMisuseForm.js +++ b/assets/js/Components/Forms/SensoryMisuseForm.js @@ -48,6 +48,7 @@ export default function SensoryMisuseForm({ const includedAttributes = ['alt', 'title', 'aria-label'] const editorRef = useRef(null) + const [sensoryErrors, setSensoryErrors] = useState([]) useEffect(() => { @@ -107,12 +108,7 @@ export default function SensoryMisuseForm({ } const traverseNode = (node) => { - - // Editor HTML is empty so node ends up empty - if(node === null) - return - - if (node.nodeType === Node.TEXT_NODE) { + if (node?.nodeType === Node.TEXT_NODE) { // Check if the text node contains any sensory words checkText(node.textContent); } diff --git a/assets/js/Components/Forms/TableHeadersForm.js b/assets/js/Components/Forms/TableHeadersForm.js index 2a56439a0..08dee3ff9 100644 --- a/assets/js/Components/Forms/TableHeadersForm.js +++ b/assets/js/Components/Forms/TableHeadersForm.js @@ -74,11 +74,24 @@ export default function TableHeadersForm({ } const removeHeaders = (table) => { + + // The table_structure_misuse rule means that when a table is marked as decorative, + // it should not contain any data-related elements, like or . All 'summary', + // 'scope', and 'headers' attributes should also be removed. + if(decorationOnly) { + table.removeAttribute('summary') + let caption = table.querySelector('caption') + if(caption) { + caption.remove() + } + } + for (let row of table.rows) { for (let cell of row.cells) { + cell.removeAttribute('scope') + cell.removeAttribute('headers') if (cell.tagName === 'TH') { let newCell = Html.renameElement(cell, 'td') - newCell.removeAttribute('scope') row.replaceChild(newCell, cell) } } diff --git a/assets/js/Components/Widgets/Combobox.css b/assets/js/Components/Widgets/Combobox.css index 528e38666..c7e06281a 100644 --- a/assets/js/Components/Widgets/Combobox.css +++ b/assets/js/Components/Widgets/Combobox.css @@ -28,7 +28,6 @@ .combo-input { background-color: var(--white); - display: block; font-size: .85em; padding: .4em .15em .4em .5em; border: 1px solid var(--border-color); diff --git a/assets/js/Components/Widgets/FixIssuesContentPreview.js b/assets/js/Components/Widgets/FixIssuesContentPreview.js index 9e7230319..f05f794ff 100644 --- a/assets/js/Components/Widgets/FixIssuesContentPreview.js +++ b/assets/js/Components/Widgets/FixIssuesContentPreview.js @@ -89,12 +89,12 @@ export default function FixIssuesContentPreview({ } const addPreviewHelperElements = (doc, errorElement) => { - if(!activeIssue || !doc || !errorElement) { + if(!activeIssue || !doc) { return doc } // If the issue edits the alt text, we need to show the auto-updating alt text preview - if (ALT_TEXT_RELATED.includes(formNameFromRule(activeIssue.scanRuleId))) { + if (ALT_TEXT_RELATED.includes(formNameFromRule(activeIssue.scanRuleId)) && errorElement) { let altText = Html.getAccessibleName(errorElement) // If there is alt text to show... diff --git a/assets/js/Services/Html.js b/assets/js/Services/Html.js index 57f03d264..d2e2c743c 100644 --- a/assets/js/Services/Html.js +++ b/assets/js/Services/Html.js @@ -488,7 +488,7 @@ export function findElementWithIssue(content, issue) { else { let errorHtml = issue?.sourceHtml || undefined if(issue.status.toString() === '1') { - errorHtml = issue?.newHtml || errorHtml + errorHtml = issue?.newHtml || undefined } if(errorHtml === undefined || errorHtml === '') { diff --git a/jest.setup.js b/jest.setup.js index 0c9b39c97..b21c6268d 100644 --- a/jest.setup.js +++ b/jest.setup.js @@ -2,7 +2,6 @@ import 'regenerator-runtime/runtime' import '@testing-library/jest-dom' import { server } from './assets/js/__tests__/mocks/server.js' -import 'whatwg-fetch'; // Establish API mocking before all tests. beforeAll(() => server.listen()) diff --git a/package.json b/package.json index e446208c8..06bbdcd0f 100644 --- a/package.json +++ b/package.json @@ -17,12 +17,10 @@ "@testing-library/user-event": "^14.6.1", "copy-webpack-plugin": "13.0.0", "jest": "^30.0.4", - "jest-environment-jsdom": "^30.0.4", "msw": "^2.10.4", "regenerator-runtime": "^0.14.1", "webpack-cli": "^5.1.4", - "webpack-notifier": "^1.15.0", - "whatwg-fetch": "^3.6.20" + "webpack-notifier": "^1.15.0" }, "license": "GPL-3.0-only", "private": true, diff --git a/src/Controller/IssuesController.php b/src/Controller/IssuesController.php index 97c657005..9c7b2a33e 100644 --- a/src/Controller/IssuesController.php +++ b/src/Controller/IssuesController.php @@ -61,13 +61,16 @@ public function saveIssue( $contentUpdated = true; // Check if new HTML is different from original HTML - if ($newHtml !== '' && ($sourceHtml === $newHtml || $issue->getPreviewHtml() === $newHtml || $issue->getNewHtml() === $newHtml)) { + if ($newHtml !== '' && + ( + $sourceHtml === $newHtml || + $issue->getPreviewHtml() === $newHtml || + $issue->getNewHtml() === $newHtml + ) + ) { $contentUpdated = false; } - $output->writeln("Review updated: ".($reviewUpdated ? 'true' : 'false')); - $output->writeln("Content updated: ".($contentUpdated ? 'true' : 'false')); - if(!$reviewUpdated && !$contentUpdated) { throw new \Exception('form.error.same_html'); } diff --git a/src/Response/ApiResponse.php b/src/Response/ApiResponse.php index 58a842af5..0b3016bbb 100644 --- a/src/Response/ApiResponse.php +++ b/src/Response/ApiResponse.php @@ -32,6 +32,10 @@ public function addMessage($msg, $severity = 'info', $timeout = 10000, $visible 'timeout' => $timeout, 'visible' => $visible, ]; + + if($severity === 'error') { + $this->addError($msg); + } } // Undocumented function