Summary: | AX: aria-level does not override implicit level on h1, h2, etc | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | James Craig <jcraig> | ||||||||||
Component: | Accessibility | Assignee: | James Craig <jcraig> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | aboxhall, apinheiro, cfleizach, commit-queue, dmazzoni, jdiggs, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
James Craig
2013-04-16 11:21:27 PDT
Created attachment 198479 [details]
patch
Comment on attachment 198479 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=198479&action=review > Source/WebCore/ChangeLog:3 > + aria-level does not override implicit level on h1, h2, etc We should prefix accessibility bugs with AX: so it's easier to know what aspect of the code their referring to > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:719 > + if (isHeading() && getAttribute(aria_levelAttr).toInt() > 0) You should cache the value of getAttribute rather than call it two times (In reply to comment #3) > We should prefix accessibility bugs with AX: so it's easier to know what aspect… Is this concern only for the ChangeLog comments? I hadn't been prefixing the bug titles b/c they're all filed in the "Accessibility" component. > You should cache the value of getAttribute rather than call it two times Okay, thanks. (In reply to comment #4) > (In reply to comment #3) > > We should prefix accessibility bugs with AX: so it's easier to know what aspect… > > Is this concern only for the ChangeLog comments? I hadn't been prefixing the bug titles b/c they're all filed in the "Accessibility" component. My preference is that all Accessibility bugs have AX: in the title. The category of the bug isn't transferred to change logs, commit logs, etc, etc > > > You should cache the value of getAttribute rather than call it two times > > Okay, thanks. Created attachment 198581 [details]
patch
Comment on attachment 198581 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=198581&action=review > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:720 > + if (isHeading() && ariaLevel > 0) we should probably do if (isHeading()) { int ariaLevel = getAttribute(aria_levelAttr).toInt(); if (ariaLevel > 0) return ariaLevel; } so that every object doesn't have to make that getAttribute call Created attachment 198589 [details]
patch with review feedback
Comment on attachment 198589 [details] patch with review feedback View in context: https://bugs.webkit.org/attachment.cgi?id=198589&action=review > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:721 > + if (ariaLevel > 0) { brackets not needed for a one line if (this will fail style-bot) > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:743 > + this extra insertion should probably be removed to reduce churn Created attachment 198606 [details]
patch with review feedback
Comment on attachment 198606 [details] patch with review feedback Clearing flags on attachment: 198606 Committed r148654: <http://trac.webkit.org/changeset/148654> All reviewed patches have been landed. Closing bug. |