[webkit-changes] [WebKit/WebKit] 87adae: AX: In rare scenarios, WebKit fails to expose the ...

Tyler Wilcock noreply at github.com
Sat Feb 18 16:44:47 PST 2023


  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 87adaeb6e0784bbc78fb88e5db1178d06957243f
      https://github.com/WebKit/WebKit/commit/87adaeb6e0784bbc78fb88e5db1178d06957243f
  Author: Tyler Wilcock <tyler_w at apple.com>
  Date:   2023-02-18 (Sat, 18 Feb 2023)

  Changed paths:
    A LayoutTests/accessibility/empty-text-under-element-cached-expected.txt
    A LayoutTests/accessibility/empty-text-under-element-cached.html
    M LayoutTests/platform/glib/TestExpectations
    M LayoutTests/platform/ios/TestExpectations
    A LayoutTests/platform/ios/accessibility/empty-text-under-element-cached-expected.txt
    A LayoutTests/platform/mac-wk1/accessibility/empty-text-under-element-cached-expected.txt
    M Source/WebCore/accessibility/AccessibilityRenderObject.cpp

  Log Message:
  -----------
  AX: In rare scenarios, WebKit fails to expose the text associated with various types of elements / nodes (text, headings)
https://bugs.webkit.org/show_bug.cgi?id=252321
rdar://102025959

Reviewed by Chris Fleizach and Andres Gonzalez.

This bug happened because of this sequence:

  1. We go to build a node change for an AXIsolatedObject.
  2. In attempting to cache some property (i.e. AXPropertyName::StringValue), we call into AccessibilityRenderObject::textUnderElement.
  3. The document needs style recalc, so this function intentionally returns an empty string to avoid
     triggering layout which may potentially destroy renderers and AX objects in use in this stackframe
  4. We cache an empty string for the property.

This patch fixes this by removing the behavior and ASSERT listed in step
3. For a long time (~8 years), this was only a debug assert. It was "upgraded"
to actually return an empty string 2 years ago in:

https://github.com/WebKit/WebKit/commit/977f75a79bc71147cd9a434241072b720096320d

There are other ways layout can be triggered from accessibility code that we already
have to protect against, so it doesn't make sense to workaround the issue this way.

* LayoutTests/platform/glib/TestExpectations:
Skip new test.
* LayoutTests/accessibility/empty-text-under-element-cached-expected.txt: Added.
* LayoutTests/accessibility/empty-text-under-element-cached.html: Added.
* LayoutTests/platform/ios/TestExpectations:
Enable new test.
* LayoutTests/platform/ios/accessibility/empty-text-under-element-cached-expected.txt: Added.
* LayoutTests/platform/mac-wk1/accessibility/empty-text-under-element-cached-expected.txt: Added.
* Source/WebCore/accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::textUnderElement const):

Canonical link: https://commits.webkit.org/260521@main




More information about the webkit-changes mailing list