Bug 130071

Summary:

AX: AccessibilityObject::invalidStatus() is incorrect when aria-invalid="undefined" or whitespace

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, mario, samuel_white, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
patch	none
patch	none
patch	none
patch with review feedback	none
patch merges conflicts with TOT	cfleizach: review+
patch to fix ATK build failure	none
previous patch -virtual	none

James Craig

Reported 2014-03-11 00:25:17 PDT

AX: AccessibilityObject::invalidStatus() is incorrect when aria-invalid="undefined" The when the string value "undefined" is used explicitly, it should be treated the same as ariaInvalid.isEmpty() // If 'false', empty or not present, it should return false. if (ariaInvalid.isEmpty() || equalIgnoringCase(ariaInvalid, invalidStatusFalse)) return invalidStatusFalse; Should be: // If 'false', empty or not present, it should return false. if (ariaInvalid.isEmpty() || equalIgnoringCase(ariaInvalid, invalidStatusFalse) || equalIgnoringCase(ariaInvalid, "undefined")) return invalidStatusFalse;

Attachments
patch (11.79 KB, patch) 2014-03-13 19:17 PDT, James Craig	no flags	Details Formatted Diff Diff
patch (11.80 KB, patch) 2014-03-13 19:33 PDT, James Craig	no flags	Details Formatted Diff Diff
patch (12.00 KB, patch) 2014-03-13 20:07 PDT, James Craig	no flags	Details Formatted Diff Diff
patch with review feedback (12.77 KB, patch) 2014-03-14 11:31 PDT, James Craig	no flags	Details Formatted Diff Diff
patch merges conflicts with TOT (12.79 KB, patch) 2014-03-14 11:45 PDT, James Craig	cfleizach: review+	Details Formatted Diff Diff
patch to fix ATK build failure (13.71 KB, patch) 2014-03-14 11:58 PDT, James Craig	no flags	Details Formatted Diff Diff
previous patch -virtual (13.71 KB, patch) 2014-03-14 13:19 PDT, James Craig	no flags	Details Formatted Diff Diff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.

James Craig

Comment 1 2014-03-13 19:12:57 PDT

<rdar://problem/16322420>

James Craig

Comment 2 2014-03-13 19:17:10 PDT

Created attachment 226639 [details] patch

James Craig

Comment 3 2014-03-13 19:30:02 PDT

[ 16%] Building CXX object Source/WebCore/CMakeFiles/WebCore.dir/inspector/InspectorResourceAgent.cpp.o /mnt/eflews/git/webkit/Source/WebCore/accessibility/AccessibilityObject.cpp: In member function 'virtual const WTF::AtomicString& WebCore::AccessibilityObject::invalidStatus() const': /mnt/eflews/git/webkit/Source/WebCore/accessibility/AccessibilityObject.cpp:1540:103: error: 'stripLeadingAndTrailingHTMLSpaces' was not declared in this scope make[2]: *** [Source/WebCore/CMakeFiles/WebCore.dir/accessibility/AccessibilityObject.cpp.o] Error 1 make[2]: *** Waiting for unfinished jobs....

James Craig

Comment 4 2014-03-13 19:33:38 PDT

Created attachment 226640 [details] patch

James Craig

Comment 5 2014-03-13 20:07:03 PDT

Created attachment 226643 [details] patch

chris fleizach

Comment 6 2014-03-14 09:24:39 PDT

Comment on attachment 226643 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=226643&action=review > Source/WebCore/ChangeLog:3 > + AX: AccessibilityObject::invalidStatus() is incorrect when aria-invalid="undefined" or whitespace i don't see undefined listed in the spec http://www.w3.org/TR/wai-aria/states_and_properties > Source/WebCore/accessibility/AccessibilityObject.cpp:1534 > + DEFINE_STATIC_LOCAL(const AtomicString, invalidStatusGrammar, ("grammar", AtomicString::ConstructFromLiteral)); there's been a desire to stop using statics for things like this. so we might as well change this style if we're going to modify this code. Let's just use ASCIILiteral("grammar") when necessary. See https://chromium.googlesource.com/chromium/blink/+/873a4a1f950d7cd7ebe27baba2c0e7e9491190f4%5E%21/#F3 for more examples

James Craig

Comment 7 2014-03-14 10:09:57 PDT

Comment on attachment 226643 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=226643&action=review >> Source/WebCore/ChangeLog:3 >> + AX: AccessibilityObject::invalidStatus() is incorrect when aria-invalid="undefined" or whitespace > > i don't see undefined listed in the spec > http://www.w3.org/TR/wai-aria/states_and_properties From http://www.w3.org/WAI/PF/aria/complete#propcharacteristic_value The "undefined" value, when allowed on a state or property, is an explicit indication that the state or property is not set. The value is used on states and properties that support tokens, and the "undefined" value is a string that is one of the allowed tokens.

James Craig

Comment 8 2014-03-14 11:31:03 PDT

Created attachment 226739 [details] patch with review feedback

James Craig

Comment 9 2014-03-14 11:45:20 PDT

Created attachment 226743 [details] patch merges conflicts with TOT

chris fleizach

Comment 10 2014-03-14 11:48:31 PDT

Comment on attachment 226743 [details] patch merges conflicts with TOT we should remove virtual from the method, since subclassers won't override this

James Craig

Comment 11 2014-03-14 11:58:18 PDT

Created attachment 226746 [details] patch to fix ATK build failure

James Craig

Comment 12 2014-03-14 13:19:43 PDT

Created attachment 226756 [details] previous patch -virtual

WebKit Commit Bot

Comment 13 2014-03-14 14:27:19 PDT

Comment on attachment 226756 [details] previous patch -virtual Rejecting attachment 226756 [details] from commit-queue. jcraig@apple.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/contributors.json. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/contributors.json by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights.

James Craig

Comment 14 2014-03-14 14:27:50 PDT

Told you, Sam. ;-)

WebKit Commit Bot

Comment 15 2014-03-14 15:02:32 PDT

Comment on attachment 226756 [details] previous patch -virtual Clearing flags on attachment: 226756 Committed r165656: <http://trac.webkit.org/changeset/165656>

WebKit Commit Bot

Comment 16 2014-03-14 15:02:36 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.