Bug 193414 – AX: Support returning relative frames for accessibility

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[chris fleizach]

Currently, when VoiceOver requests a frame of an element, WebKit has to synchronously send a message back to Safari to provide the final conversions.

This has some drawbacks

1) It blocks the WebProcess rendering thread while this call is made
2) It potentially does unnecessary work, if VO doesn't actually need the full frame (perhaps it's just fetching a parent to check a property)
3) It makes it hard to cache the frame, because the containing window may change or rotate and we'd have to empty the cache on those events

We can instead offer an AXRelativeFrame which is in "Page" space. The final conversion can be done by VoiceOver only when it needs it

[Radar WebKit Bug Importer]

<rdar://problem/47268501>

[chris fleizach]

Created attachment 359097 [details]
patch

[EWS Watchlist]

Attachment 359097 [details] did not pass style-queue:


ERROR: Source/WebKit/UIProcess/API/mac/WKView.mm:815:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/mac/WKView.mm:822:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4025:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4032:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 4 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 359097 [details]
patch

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

New failing tests:
accessibility/mac/relative-frame.html

[EWS Watchlist]

Created attachment 359104 [details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2

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

[EWS Watchlist]

Comment on attachment 359097 [details]
patch

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

New failing tests:
accessibility/plugin.html
accessibility/mac/bounds-for-range.html
accessibility/image-link.html
accessibility/table-attributes.html
accessibility/mac/internal-link-anchors.html
accessibility/mac/document-links.html
accessibility/table-with-rules.html
accessibility/table-one-cell.html
accessibility/table-sections.html
accessibility/table-cells.html
accessibility/image-map2.html
accessibility/table-cell-spans.html
accessibility/mac/aria-columnrowheaders.html
accessibility/transformed-element.html
accessibility/math-multiscript-attributes.html
accessibility/internal-link-anchors2.html
accessibility/lists.html

[EWS Watchlist]

Created attachment 359106 [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 359097 [details]
patch

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

New failing tests:
accessibility/plugin.html
accessibility/mac/bounds-for-range.html
accessibility/image-link.html
accessibility/mac/internal-link-anchors.html
accessibility/mac/document-links.html
accessibility/table-with-rules.html
accessibility/math-multiscript-attributes.html
accessibility/table-one-cell.html
accessibility/table-cells.html
accessibility/table-sections.html
accessibility/table-cell-spans.html
accessibility/image-map2.html
accessibility/lists.html
accessibility/transformed-element.html
accessibility/mac/aria-columnrowheaders.html
accessibility/internal-link-anchors2.html
accessibility/table-attributes.html

[EWS Watchlist]

Created attachment 359117 [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

[EWS Watchlist]

Comment on attachment 359097 [details]
patch

Attachment 359097 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/10753841

New failing tests:
webanimations/leak-document-with-web-animation.html

[EWS Watchlist]

Created attachment 359134 [details]
Archive of layout-test-results from ews204 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews204  Port: win-future  Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit

[chris fleizach]

Created attachment 359275 [details]
patch

[chris fleizach]

Created attachment 359276 [details]
patch

[EWS Watchlist]

Attachment 359276 [details] did not pass style-queue:


ERROR: Source/WebKit/UIProcess/API/mac/WKView.mm:815:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/mac/WKView.mm:822:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4025:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4032:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 4 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[chris fleizach]

Created attachment 359335 [details]
patch

[EWS Watchlist]

Attachment 359335 [details] did not pass style-queue:


ERROR: Source/WebKit/UIProcess/API/mac/WKView.mm:815:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/mac/WKView.mm:822:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4026:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4033:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 4 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[chris fleizach]

Created attachment 359405 [details]
patch

[EWS Watchlist]

Attachment 359405 [details] did not pass style-queue:


ERROR: Source/WebKit/UIProcess/API/mac/WKView.mm:815:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/mac/WKView.mm:822:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4026:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4033:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 4 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[chris fleizach]

Created attachment 359408 [details]
patch

[EWS Watchlist]

Attachment 359408 [details] did not pass style-queue:


ERROR: Source/WebKit/UIProcess/API/mac/WKView.mm:815:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/mac/WKView.mm:822:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4026:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4033:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 4 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[chris fleizach]

Created attachment 359421 [details]
patch

[EWS Watchlist]

Attachment 359421 [details] did not pass style-queue:


ERROR: Source/WebKit/UIProcess/API/mac/WKView.mm:815:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/mac/WKView.mm:822:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4026:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4033:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 4 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 359421 [details]
patch

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

New failing tests:
accessibility/plugin.html
accessibility/mac/bounds-for-range.html
accessibility/image-link.html
accessibility/table-attributes.html
accessibility/mac/internal-link-anchors.html
accessibility/internal-link-anchors2.html
accessibility/mac/document-links.html
accessibility/table-with-rules.html
accessibility/mac/document-attributes.html
accessibility/table-one-cell.html
accessibility/table-sections.html
accessibility/table-cells.html
accessibility/image-map2.html
accessibility/table-cell-spans.html
accessibility/mac/aria-columnrowheaders.html
accessibility/transformed-element.html
accessibility/math-multiscript-attributes.html
accessibility/parent-delete.html
accessibility/lists.html

[EWS Watchlist]

Created attachment 359423 [details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2

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

[chris fleizach]

Created attachment 359434 [details]
patch

[chris fleizach]

Created attachment 359435 [details]
patch

[EWS Watchlist]

Attachment 359435 [details] did not pass style-queue:


ERROR: Source/WebKit/UIProcess/API/mac/WKView.mm:815:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/mac/WKView.mm:822:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4026:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4033:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 4 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[zalan]

Comment on attachment 359435 [details]
patch

nit; when I move some old code around, I tend to modernize it (some lines could use a bit of auto)

[chris fleizach]

(In reply to zalan from comment #28)
> Comment on attachment 359435 [details]
> patch
> 
> nit; when I move some old code around, I tend to modernize it (some lines
> could use a bit of auto)

will do. thanks

[chris fleizach]

Created attachment 359636 [details]
landing patch

[EWS Watchlist]

Attachment 359636 [details] did not pass style-queue:


ERROR: Source/WebKit/UIProcess/API/mac/WKView.mm:815:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/mac/WKView.mm:822:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4047:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4054:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 4 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 359636 [details]
landing patch

Clearing flags on attachment: 359636

Committed r240209: <https://trac.webkit.org/changeset/240209>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Michael Catanzaro]

Reverted r240209 for reason:

Broke GTK/WPE injected bundle

Committed r240212: <https://trac.webkit.org/changeset/240212>

[Michael Catanzaro]

Apologies for the rollout. This broke the GTK/WPE test bots (and probably other ports too). Problem is:

** (WebKitWebProcess:68170): WARNING **: 03:40:34.935: Error loading the injected bundle (/home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/Release/lib/libTestRunnerInjectedBundle.so): /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/Release/lib/libTestRunnerInjectedBundle.so: undefined symbol: _ZN3WTR22AccessibilityUIElement33stringDescriptionOfAttributeValueEP14OpaqueJSString

Would it be reasonable to add an empty default implementation for non-Apple ports, or do we need more work here?

[chris fleizach]

(In reply to Michael Catanzaro from comment #35)
> Apologies for the rollout. This broke the GTK/WPE test bots (and probably
> other ports too). Problem is:
> 
> ** (WebKitWebProcess:68170): WARNING **: 03:40:34.935: Error loading the
> injected bundle
> (/home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/Release/
> lib/libTestRunnerInjectedBundle.so):
> /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/Release/
> lib/libTestRunnerInjectedBundle.so: undefined symbol:
> _ZN3WTR22AccessibilityUIElement33stringDescriptionOfAttributeValueEP14OpaqueJ
> SString
> 
> Would it be reasonable to add an empty default implementation for non-Apple
> ports, or do we need more work here?

That would be reasonable. Why doesn't this fail in EWS? It seems completely unreasonable that I can pass a patch in EWS and have it rolled out do to an unknowable dependency?

Could you instead work to put in a fix rather than rollout? How will I even test a GTK change if the EWS bots won't build it?

[Michael Catanzaro]

(In reply to chris fleizach from comment #36)
> That would be reasonable. Why doesn't this fail in EWS? It seems completely
> unreasonable that I can pass a patch in EWS and have it rolled out do to an
> unknowable dependency?

Problem is it did build, it just fails to dlopen at runtime. Only Mac EWS run tests. :(

> Could you instead work to put in a fix rather than rollout? How will I even
> test a GTK change if the EWS bots won't build it?

I don't understand the code, so I don't know if that would be correct. If just a default implementation would suffice, I would just try adding one and relanding.

[chris fleizach]

Created attachment 359651 [details]
landing patch

[EWS Watchlist]

Attachment 359651 [details] did not pass style-queue:


ERROR: Source/WebKit/UIProcess/API/mac/WKView.mm:815:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/mac/WKView.mm:822:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4047:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4054:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 4 in 23 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 359651 [details]
landing patch

Attachment 359651 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10822441

New failing tests:
imported/w3c/web-platform-tests/webrtc/simplecall.https.html

[EWS Watchlist]

Created attachment 359656 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[WebKit Commit Bot]

Comment on attachment 359651 [details]
landing patch

Clearing flags on attachment: 359651

Committed r240219: <https://trac.webkit.org/changeset/240219>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Michael Catanzaro]

That worked, thanks!