Bug 199216 – Enhance support of aria-haspopup per ARIA 1.1 specification.

• Component: New Bugs
• Status: RESOLVED FIXED

[Andres Gonzalez]

Enhance support of aria-haspopup per ARIA 1.1 specification.

[Andres Gonzalez]

Created attachment 372892 [details]
Patch

[chris fleizach]

Comment on attachment 372892 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=372892&action=review

> Source/WebCore/accessibility/AccessibilityObject.cpp:2767
> +    if (equalLettersIgnoringASCIICase(hasPopup, "dialog"))

should we make a static HashSet of valid values and then just check based on passing in the popupAttr.lowercaseString() ?

[EWS Watchlist]

Comment on attachment 372892 [details]
Patch

Attachment 372892 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/12576757

New failing tests:
accessibility/button-with-aria-haspopup-role.html

[EWS Watchlist]

Created attachment 372895 [details]
Archive of layout-test-results from ews103 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 372892 [details]
Patch

Attachment 372892 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/12576854

New failing tests:
accessibility/button-with-aria-haspopup-role.html

[EWS Watchlist]

Created attachment 372899 [details]
Archive of layout-test-results from ews112 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[Andres Gonzalez]

Created attachment 372927 [details]
Patch

[Andres Gonzalez]

Created attachment 372997 [details]
Patch

[Andres Gonzalez]

(In reply to chris fleizach from comment #2)
> Comment on attachment 372892 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=372892&action=review
> 
> > Source/WebCore/accessibility/AccessibilityObject.cpp:2767
> > +    if (equalLettersIgnoringASCIICase(hasPopup, "dialog"))
> 
> should we make a static HashSet of valid values and then just check based on
> passing in the popupAttr.lowercaseString() ?

Thanks for the suggestion. Using HashSet in latest patch. Doesn't buy us much in terms of performance, but code is more elegant and compact. Still have to handle a couple of cases separately. Was looking for an equivalent to boost::hana strings in WTF, but couldn't find it. boost::hana would be ideal for cases like this because the hana strings are as efficient as literals but the set also have the searching capabilities like contains.

Also fixed the DumpRenderTree build and test in latest upload.

[chris fleizach]

Comment on attachment 372997 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=372997&action=review

> Source/WebCore/accessibility/AccessibilityObject.cpp:2767
> +    AtomString hasPopup = getAttribute(aria_haspopupAttr).convertToASCIILowercase();

auto popupValue

> Source/WebCore/accessibility/AccessibilityObject.cpp:2774
> +    // aria-haspopup specification states that true must be treated as menu

end sentence with a period

> Tools/ChangeLog:20
> +        Enhance support of aria-haspopup per ARIA 1.1 specification.

remove duplicate change log entry

[Andres Gonzalez]

Created attachment 373032 [details]
Patch

[chris fleizach]

Comment on attachment 373032 [details]
Patch

looks like this win failure is related to the patch

   Creating library C:/cygwin/home/buildbot/WebKit/WebKitBuild/Release/lib32/DumpRenderTreeLib.lib and object C:/cygwin/home/buildbot/WebKit/WebKitBuild/Release/lib32/DumpRenderTreeLib.exp
AccessibilityUIElement.obj : error LNK2019: unresolved external symbol "public: class JSRetainPtr<struct OpaqueJSString *> __thiscall AccessibilityUIElement::popupValue(void)const " (?popupValue@AccessibilityUIElement@@QBE?AV?$JSRetainPtr@PAUOpaqueJSString@@@@XZ) referenced in function "struct OpaqueJSValue const * __cdecl getPopupValueCallback(struct OpaqueJSContext const *,struct OpaqueJSValue *,struct OpaqueJSString *,struct OpaqueJSValue const * *)" (?getPopupValueCallback@@YAPBUOpaqueJSValue@@PBUOpaqueJSContext@@PAU1@PAUOpaqueJSString@@PAPBU1@@Z) [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Tools\DumpRenderTree\DumpRenderTreeLib.vcxproj]

[Andres Gonzalez]

Created attachment 373193 [details]
Patch

[Andres Gonzalez]

(In reply to chris fleizach from comment #12)
> Comment on attachment 373032 [details]
> Patch
> 
> looks like this win failure is related to the patch
> 
>    Creating library
> C:/cygwin/home/buildbot/WebKit/WebKitBuild/Release/lib32/DumpRenderTreeLib.
> lib and object
> C:/cygwin/home/buildbot/WebKit/WebKitBuild/Release/lib32/DumpRenderTreeLib.
> exp
> AccessibilityUIElement.obj : error LNK2019: unresolved external symbol
> "public: class JSRetainPtr<struct OpaqueJSString *> __thiscall
> AccessibilityUIElement::popupValue(void)const "
> (?popupValue@AccessibilityUIElement@@QBE?AV?$JSRetainPtr@PAUOpaqueJSString@@@
> @XZ) referenced in function "struct OpaqueJSValue const * __cdecl
> getPopupValueCallback(struct OpaqueJSContext const *,struct OpaqueJSValue
> *,struct OpaqueJSString *,struct OpaqueJSValue const * *)"
> (?getPopupValueCallback@@YAPBUOpaqueJSValue@@PBUOpaqueJSContext@@PAU1@PAUOpaq
> ueJSString@@PAPBU1@@Z)
> [C:
> \cygwin\home\buildbot\WebKit\WebKitBuild\Release\Tools\DumpRenderTree\DumpRen
> derTreeLib.vcxproj]

Fix for DRT Win build in latest patch upload.

[chris fleizach]

wincairo failure now

  Creating library lib64\TestRunnerInjectedBundle.lib and object lib64\TestRunnerInjectedBundle.exp
JSAccessibilityUIElement.cpp.obj : error LNK2019: unresolved external symbol "public: class JSRetainPtr<struct OpaqueJSString *> __cdecl WTR::AccessibilityUIElement::popupValue(void)const " (?popupValue@AccessibilityUIElement@WTR@@QEBA?AV?$JSRetainPtr@PEAUOpaqueJSString@@@@XZ) referenced in function "private: static struct OpaqueJSValue const * __cdecl WTR::JSAccessibilityUIElement::popupValue(struct OpaqueJSContext const *,struct OpaqueJSValue *,struct OpaqueJSString *,struct OpaqueJSValue const * *)" (?popupValue@JSAccessibilityUIElement@WTR@@CAPEBUOpaqueJSValue@@PEBUOpaqueJSContext@@PEAU3@PEAUOpaqueJSString@@PEAPEBU3@@Z)
bin64\TestRunnerInjectedBundle.dll : fatal error LNK1120: 1 unresolved externals

[Andres Gonzalez]

Created attachment 373197 [details]
Patch

[Andres Gonzalez]

(In reply to chris fleizach from comment #15)
> wincairo failure now
> 
>   Creating library lib64\TestRunnerInjectedBundle.lib and object
> lib64\TestRunnerInjectedBundle.exp
> JSAccessibilityUIElement.cpp.obj : error LNK2019: unresolved external symbol
> "public: class JSRetainPtr<struct OpaqueJSString *> __cdecl
> WTR::AccessibilityUIElement::popupValue(void)const "
> (?popupValue@AccessibilityUIElement@WTR@@QEBA?AV?$JSRetainPtr@PEAUOpaqueJSStr
> ing@@@@XZ) referenced in function "private: static struct OpaqueJSValue
> const * __cdecl WTR::JSAccessibilityUIElement::popupValue(struct
> OpaqueJSContext const *,struct OpaqueJSValue *,struct OpaqueJSString
> *,struct OpaqueJSValue const * *)"
> (?popupValue@JSAccessibilityUIElement@WTR@@CAPEBUOpaqueJSValue@@PEBUOpaqueJSC
> ontext@@PEAU3@PEAUOpaqueJSString@@PEAPEBU3@@Z)
> bin64\TestRunnerInjectedBundle.dll : fatal error LNK1120: 1 unresolved
> externals

Fixed now. Perhaps a script to add all the no implemented methods to DRT and WTR for other platforms would be worth the trouble...

[WebKit Commit Bot]

Comment on attachment 373197 [details]
Patch

Clearing flags on attachment: 373197

Committed r246958: <https://trac.webkit.org/changeset/246958>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/52433622>

[Truitt Savell]

The changes in https://trac.webkit.org/changeset/246958/webkit

broke inspector/dom/getAccessibilityPropertiesForNode.html

History:
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=inspector%2Fdom%2FgetAccessibilityPropertiesForNode.html

Diff:
--- /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/inspector/dom/getAccessibilityPropertiesForNode-expected.txt
+++ /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/inspector/dom/getAccessibilityPropertiesForNode-actual.txt
@@ -296,7 +296,6 @@
     focused: false
     ownedNodeIds.length: 1
     required: false
-    isPopUpButton: true
 
 <select>
     <option>FOO</option>


Is this possibly a rebasline?

[Truitt Savell]

Reverted r246958 for reason:

Broke inspector/dom/getAccessibilityPropertiesForNode.html

Committed r247019: <https://trac.webkit.org/changeset/247019>

[Ryan Haddad]

This passed EWS because the test is marked as a flaky timeout:
LayoutTests/platform/mac/TestExpectations:1063:webkit.org/b/148636 inspector/dom/getAccessibilityPropertiesForNode.html [ Pass Timeout ]

EWS runs tests with the '--skip-failing-tests' argument, which appears to apply to flaky tests. Because of that, this test wasn't run at all pre-commit.

[Andres Gonzalez]

Created attachment 373300 [details]
Patch

[Andres Gonzalez]

inspector/dom/getAccessibilityPropertiesForNode.html caught an issue with my previous patch concerning comboboxes. This latest patch addresses that issue. inspector/dom/getAccessibilityPropertiesForNode.html may still be a flaky test, though.

[Andres Gonzalez]

Created attachment 373325 [details]
Patch

[Andres Gonzalez]

New patch with all changes including fix for inspector/dom/getAccessibilityPropertiesForNode.html.

[WebKit Commit Bot]

Comment on attachment 373325 [details]
Patch

Clearing flags on attachment: 373325

Committed r247071: <https://trac.webkit.org/changeset/247071>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.