- Revision
- 287038
- Author
- [email protected]
- Date
- 2021-12-14 11:59:26 -0800 (Tue, 14 Dec 2021)
Log Message
Web Inspector: test webpage keeps reloading when Inspector is open
https://bugs.webkit.org/show_bug.cgi?id=234076
Reviewed by Chris Fleizach.
Source/WebCore:
https://trac.webkit.org/changeset/284335/webkit introduced the
potential for infinite recursion in AccessibilityObjectMac::shouldIgnoreGroup:
>From accessibilityIsIgnored to computeAccessibilityIsIgnored to
defaultObjectInclusion to accessibilityPlatformIncludesObject to
shouldIgnoreGroup to accessibilityText to titleElementText to
exposesTitleUIElement to accessibilityIsIgnored to ...
shouldIgnoreGroup returns `true` if the group has one static text
child with the same content as the group's AX text. We fix the
recursion by making this check more conservative.
Rather than checking all of the AX text (which includes title text, alt text, help text,
visible text, placeholder text), only check the group's alt text and help text. This
accomplishes the original problem statement laid out in
https://bugs.webkit.org/show_bug.cgi?id=169924#c0 while avoiding
infinite recursion, as nothing in AccessibilityNodeObject::alternativeText or
AccessibilityNodeObject::helpText calls accessibilityIsIgnored.
Test: accessibility/mac/ignore-redundant-groups-crash.html
* accessibility/AccessibilityNodeObject.h:
Move alternativeText and helpText method definitions from private to public
so they can be called by the static shouldIgnoreGroup method in AccessibilityObjectMac.
* accessibility/mac/AccessibilityObjectMac.mm:
(WebCore::shouldIgnoreGroup):
Check only alt text and help text rather than all
AX text when deciding if a group should be ignored.
LayoutTests:
This patch fixes the potential for infinite recursion introduced by
https://trac.webkit.org/changeset/284335/webkit. See corresponding
Source/WebCore/ChangeLog for full details.
* accessibility/mac/ignore-redundant-accessibility-text-groups-expected.txt:
* accessibility/mac/ignore-redundant-accessibility-text-groups.html:
Add an aria-describedby testcase.
* accessibility/mac/ignore-redundant-groups-crash-expected.txt: Added.
* accessibility/mac/ignore-redundant-groups-crash.html:
Added. Exercises the crash testcase raised in
https://bugs.webkit.org/show_bug.cgi?id=234076 along with other
branches in AccessibilityNodeObject::alternativeText to prevent future
problems.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (287037 => 287038)
--- trunk/LayoutTests/ChangeLog 2021-12-14 19:48:28 UTC (rev 287037)
+++ trunk/LayoutTests/ChangeLog 2021-12-14 19:59:26 UTC (rev 287038)
@@ -1,3 +1,25 @@
+2021-12-14 Tyler Wilcock <[email protected]>
+
+ Web Inspector: test webpage keeps reloading when Inspector is open
+ https://bugs.webkit.org/show_bug.cgi?id=234076
+
+ Reviewed by Chris Fleizach.
+
+ This patch fixes the potential for infinite recursion introduced by
+ https://trac.webkit.org/changeset/284335/webkit. See corresponding
+ Source/WebCore/ChangeLog for full details.
+
+ * accessibility/mac/ignore-redundant-accessibility-text-groups-expected.txt:
+ * accessibility/mac/ignore-redundant-accessibility-text-groups.html:
+ Add an aria-describedby testcase.
+
+ * accessibility/mac/ignore-redundant-groups-crash-expected.txt: Added.
+ * accessibility/mac/ignore-redundant-groups-crash.html:
+ Added. Exercises the crash testcase raised in
+ https://bugs.webkit.org/show_bug.cgi?id=234076 along with other
+ branches in AccessibilityNodeObject::alternativeText to prevent future
+ problems.
+
2021-12-14 Brandon Stewart <[email protected]>
Add test case for execCommand to verify inserting an ordered list and deletion
Modified: trunk/LayoutTests/accessibility/mac/ignore-redundant-accessibility-text-groups-expected.txt (287037 => 287038)
--- trunk/LayoutTests/accessibility/mac/ignore-redundant-accessibility-text-groups-expected.txt 2021-12-14 19:48:28 UTC (rev 287037)
+++ trunk/LayoutTests/accessibility/mac/ignore-redundant-accessibility-text-groups-expected.txt 2021-12-14 19:59:26 UTC (rev 287038)
@@ -7,6 +7,7 @@
PASS !titleGroup is true
PASS !ariaLabelDiv is true
PASS !titleDiv is true
+PASS !ariaDescribedByDiv is true
PASS typeof clickHandlerGroup is 'object'
PASS typeof clickHandlerDiv is 'object'
PASS resultElement.role is 'AXRole: AXStaticText'
@@ -13,17 +14,27 @@
PASS resultElement.stringValue is 'AXValue: Blue cheese'
PASS resultElement.role is 'AXRole: AXStaticText'
PASS resultElement.stringValue is 'AXValue: Oranges'
-PASS contentContainer.childrenCount is 6
+PASS resultElement.role is 'AXRole: AXStaticText'
+PASS resultElement.stringValue is 'AXValue: Jello'
+PASS resultElement.role is 'AXRole: AXStaticText'
+PASS resultElement.stringValue is 'AXValue: Broccoli'
+PASS contentContainer.childrenCount is 8
PASS contentContainer.childAtIndex(0).stringValue is 'AXValue: Blue cheese'
PASS contentContainer.childAtIndex(0).role is 'AXRole: AXStaticText'
PASS contentContainer.childAtIndex(1).stringValue is 'AXValue: Oranges'
PASS contentContainer.childAtIndex(1).role is 'AXRole: AXStaticText'
-PASS contentContainer.childAtIndex(2).role is 'AXRole: AXGroup'
-PASS contentContainer.childAtIndex(3).stringValue is 'AXValue: Jello'
+PASS contentContainer.childAtIndex(2).stringValue is 'AXValue: Jello'
+PASS contentContainer.childAtIndex(2).role is 'AXRole: AXStaticText'
+PASS contentContainer.childAtIndex(3).stringValue is 'AXValue: Broccoli'
PASS contentContainer.childAtIndex(3).role is 'AXRole: AXStaticText'
-PASS contentContainer.childAtIndex(4).stringValue is 'AXValue: Broccoli'
-PASS contentContainer.childAtIndex(4).role is 'AXRole: AXStaticText'
-PASS contentContainer.childAtIndex(5).role is 'AXRole: AXGroup'
+Verifying #describer
+PASS contentContainer.childAtIndex(4).role is 'AXRole: AXGroup'
+PASS contentContainer.childAtIndex(5).stringValue is 'AXValue: Cheesecake'
+PASS contentContainer.childAtIndex(5).role is 'AXRole: AXStaticText'
+Verifying #click-handler-div
+PASS contentContainer.childAtIndex(6).role is 'AXRole: AXGroup'
+Verifying #click-handler-group
+PASS contentContainer.childAtIndex(7).role is 'AXRole: AXGroup'
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/accessibility/mac/ignore-redundant-accessibility-text-groups.html (287037 => 287038)
--- trunk/LayoutTests/accessibility/mac/ignore-redundant-accessibility-text-groups.html 2021-12-14 19:48:28 UTC (rev 287037)
+++ trunk/LayoutTests/accessibility/mac/ignore-redundant-accessibility-text-groups.html 2021-12-14 19:59:26 UTC (rev 287038)
@@ -16,10 +16,6 @@
Oranges
</div>
- <div id="click-handler-group" role="group" aria-label="Group click handler" _onclick_="emptyClickHandler()">
- Group click handler
- </div>
-
<!-- Also test role-less generic divs. -->
<div id="aria-label-div" aria-label="Jello">
Jello
@@ -29,9 +25,18 @@
Broccoli
</div>
+ <div id="describer">Cheesecake</div>
+ <div id="aria-describedby-div" aria-describedby="describer">
+ Cheesecake
+ </div>
+
<div id="click-handler-div" aria-label="Div click handler" _onclick_="emptyClickHandler()">
Div click handler
</div>
+
+ <div id="click-handler-group" role="group" aria-label="Group click handler" _onclick_="emptyClickHandler()">
+ Group click handler
+ </div>
</div>
<script>
@@ -39,7 +44,6 @@
const emptyClickHandler = () => {};
if (window.accessibilityController) {
-
var contentContainer = accessibilityController.accessibleElementById("content");
var ariaLabelGroup = accessibilityController.accessibleElementById("aria-label-group");
@@ -49,6 +53,7 @@
var ariaLabelDiv = accessibilityController.accessibleElementById("aria-label-div");
var clickHandlerDiv = accessibilityController.accessibleElementById("click-handler-div");
var titleDiv = accessibilityController.accessibleElementById("title-div");
+ var ariaDescribedByDiv = accessibilityController.accessibleElementById("aria-describedby-div");
// We shouldn't be able to get an accessible element for these groups because they should be ignored.
shouldBeTrue("!ariaLabelGroup");
@@ -55,6 +60,7 @@
shouldBeTrue("!titleGroup");
shouldBeTrue("!ariaLabelDiv");
shouldBeTrue("!titleDiv");
+ shouldBeTrue("!ariaDescribedByDiv");
// But any group with an event handler should always be exposed.
shouldBe("typeof clickHandlerGroup", "'object'");
shouldBe("typeof clickHandlerDiv", "'object'");
@@ -68,8 +74,16 @@
shouldBe("resultElement.role", "'AXRole: AXStaticText'");
shouldBe("resultElement.stringValue", "'AXValue: Oranges'");
+ resultElement = contentContainer.uiElementForSearchPredicate(resultElement, true, "AXAnyTypeSearchKey", "", false);
+ shouldBe("resultElement.role", "'AXRole: AXStaticText'");
+ shouldBe("resultElement.stringValue", "'AXValue: Jello'");
+
+ resultElement = contentContainer.uiElementForSearchPredicate(resultElement, true, "AXAnyTypeSearchKey", "", false);
+ shouldBe("resultElement.role", "'AXRole: AXStaticText'");
+ shouldBe("resultElement.stringValue", "'AXValue: Broccoli'");
+
// Ensure the only accessible content exposed via `children` is the text elements and event handler groups.
- shouldBe("contentContainer.childrenCount", "6");
+ shouldBe("contentContainer.childrenCount", "8");
shouldBe("contentContainer.childAtIndex(0).stringValue", "'AXValue: Blue cheese'");
shouldBe("contentContainer.childAtIndex(0).role", "'AXRole: AXStaticText'");
@@ -76,16 +90,23 @@
shouldBe("contentContainer.childAtIndex(1).stringValue", "'AXValue: Oranges'");
shouldBe("contentContainer.childAtIndex(1).role", "'AXRole: AXStaticText'");
- shouldBe("contentContainer.childAtIndex(2).role", "'AXRole: AXGroup'");
+ shouldBe("contentContainer.childAtIndex(2).stringValue", "'AXValue: Jello'");
+ shouldBe("contentContainer.childAtIndex(2).role", "'AXRole: AXStaticText'");
- shouldBe("contentContainer.childAtIndex(3).stringValue", "'AXValue: Jello'");
+ shouldBe("contentContainer.childAtIndex(3).stringValue", "'AXValue: Broccoli'");
shouldBe("contentContainer.childAtIndex(3).role", "'AXRole: AXStaticText'");
- shouldBe("contentContainer.childAtIndex(4).stringValue", "'AXValue: Broccoli'");
- shouldBe("contentContainer.childAtIndex(4).role", "'AXRole: AXStaticText'");
+ debug("Verifying #describer")
+ shouldBe("contentContainer.childAtIndex(4).role", "'AXRole: AXGroup'");
- shouldBe("contentContainer.childAtIndex(5).role", "'AXRole: AXGroup'");
+ shouldBe("contentContainer.childAtIndex(5).stringValue", "'AXValue: Cheesecake'");
+ shouldBe("contentContainer.childAtIndex(5).role", "'AXRole: AXStaticText'");
+ debug("Verifying #click-handler-div")
+ shouldBe("contentContainer.childAtIndex(6).role", "'AXRole: AXGroup'");
+ debug("Verifying #click-handler-group")
+ shouldBe("contentContainer.childAtIndex(7).role", "'AXRole: AXGroup'");
+
document.getElementById("content").style.visibility = "hidden";
}
</script>
Added: trunk/LayoutTests/accessibility/mac/ignore-redundant-groups-crash-expected.txt (0 => 287038)
--- trunk/LayoutTests/accessibility/mac/ignore-redundant-groups-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/accessibility/mac/ignore-redundant-groups-crash-expected.txt 2021-12-14 19:59:26 UTC (rev 287038)
@@ -0,0 +1,14 @@
+This test passes if WebKit doesn't crash trying to compute whether the elements on this page should be ignored.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS accessibilityController.accessibleElementById('fieldset').role is 'AXRole: AXGroup'
+PASS accessibilityController.accessibleElementById('fieldset-with-legend').role is 'AXRole: AXGroup'
+PASS accessibilityController.accessibleElementById('fieldset-with-legend').role is 'AXRole: AXGroup'
+PASS accessibilityController.accessibleElementById('figure').role is 'AXRole: AXGroup'
+PASS accessibilityController.accessibleElementById('figure-with-explicit-group-role').role is 'AXRole: AXGroup'
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/accessibility/mac/ignore-redundant-groups-crash.html (0 => 287038)
--- trunk/LayoutTests/accessibility/mac/ignore-redundant-groups-crash.html (rev 0)
+++ trunk/LayoutTests/accessibility/mac/ignore-redundant-groups-crash.html 2021-12-14 19:59:26 UTC (rev 287038)
@@ -0,0 +1,43 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+<script src=""
+</head>
+<body>
+
+<div id="content">
+ <fieldset id="fieldset">Text</fieldset>
+
+ <legend>Text</legend>
+ <fieldset id="fieldset-with-legend">Text</fieldset>
+
+ <figure id="figure">
+ <img src="" alt="Cake">
+ <figcaption>Cake</figcaption>
+ </figure>
+
+ <figure id="figure-with-explicit-group-role" role="group">
+ <img src="" alt="Cake">
+ <figcaption>Cake</figcaption>
+ </figure>
+</div>
+
+<script>
+ description("This test passes if WebKit doesn't crash trying to compute whether the elements on this page should be ignored.")
+
+ if (window.accessibilityController) {
+ shouldBe("accessibilityController.accessibleElementById('fieldset').role", "'AXRole: AXGroup'");
+
+ shouldBe("accessibilityController.accessibleElementById('fieldset-with-legend').role", "'AXRole: AXGroup'");
+ shouldBe("accessibilityController.accessibleElementById('fieldset-with-legend').role", "'AXRole: AXGroup'");
+
+ shouldBe("accessibilityController.accessibleElementById('figure').role", "'AXRole: AXGroup'");
+ shouldBe("accessibilityController.accessibleElementById('figure-with-explicit-group-role').role", "'AXRole: AXGroup'");
+
+ document.getElementById("content").style.visibility = "hidden";
+ }
+</script>
+</body>
+</html>
+
Modified: trunk/Source/WebCore/ChangeLog (287037 => 287038)
--- trunk/Source/WebCore/ChangeLog 2021-12-14 19:48:28 UTC (rev 287037)
+++ trunk/Source/WebCore/ChangeLog 2021-12-14 19:59:26 UTC (rev 287038)
@@ -1,3 +1,39 @@
+2021-12-14 Tyler Wilcock <[email protected]>
+
+ Web Inspector: test webpage keeps reloading when Inspector is open
+ https://bugs.webkit.org/show_bug.cgi?id=234076
+
+ Reviewed by Chris Fleizach.
+
+ https://trac.webkit.org/changeset/284335/webkit introduced the
+ potential for infinite recursion in AccessibilityObjectMac::shouldIgnoreGroup:
+
+ From accessibilityIsIgnored to computeAccessibilityIsIgnored to
+ defaultObjectInclusion to accessibilityPlatformIncludesObject to
+ shouldIgnoreGroup to accessibilityText to titleElementText to
+ exposesTitleUIElement to accessibilityIsIgnored to ...
+
+ shouldIgnoreGroup returns `true` if the group has one static text
+ child with the same content as the group's AX text. We fix the
+ recursion by making this check more conservative.
+
+ Rather than checking all of the AX text (which includes title text, alt text, help text,
+ visible text, placeholder text), only check the group's alt text and help text. This
+ accomplishes the original problem statement laid out in
+ https://bugs.webkit.org/show_bug.cgi?id=169924#c0 while avoiding
+ infinite recursion, as nothing in AccessibilityNodeObject::alternativeText or
+ AccessibilityNodeObject::helpText calls accessibilityIsIgnored.
+
+ Test: accessibility/mac/ignore-redundant-groups-crash.html
+
+ * accessibility/AccessibilityNodeObject.h:
+ Move alternativeText and helpText method definitions from private to public
+ so they can be called by the static shouldIgnoreGroup method in AccessibilityObjectMac.
+ * accessibility/mac/AccessibilityObjectMac.mm:
+ (WebCore::shouldIgnoreGroup):
+ Check only alt text and help text rather than all
+ AX text when deciding if a group should be ignored.
+
2021-12-14 Alan Bujtas <[email protected]>
[LFC][IFC] Take inline axis direction into account when computing horizontal alignment offset
Modified: trunk/Source/WebCore/accessibility/AccessibilityNodeObject.h (287037 => 287038)
--- trunk/Source/WebCore/accessibility/AccessibilityNodeObject.h 2021-12-14 19:48:28 UTC (rev 287037)
+++ trunk/Source/WebCore/accessibility/AccessibilityNodeObject.h 2021-12-14 19:59:26 UTC (rev 287038)
@@ -111,6 +111,8 @@
String helpText() const override;
String title() const override;
String text() const override;
+ void alternativeText(Vector<AccessibilityText>&) const;
+ void helpText(Vector<AccessibilityText>&) const;
String stringValue() const override;
SRGBA<uint8_t> colorValue() const override;
String ariaLabeledByAttribute() const override;
@@ -180,9 +182,7 @@
private:
bool isAccessibilityNodeObject() const final { return true; }
void accessibilityText(Vector<AccessibilityText>&) const override;
- void alternativeText(Vector<AccessibilityText>&) const;
void visibleText(Vector<AccessibilityText>&) const;
- void helpText(Vector<AccessibilityText>&) const;
String alternativeTextForWebArea() const;
void ariaLabeledByText(Vector<AccessibilityText>&) const;
bool computeAccessibilityIsIgnored() const override;
Modified: trunk/Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm (287037 => 287038)
--- trunk/Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm 2021-12-14 19:48:28 UTC (rev 287037)
+++ trunk/Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm 2021-12-14 19:59:26 UTC (rev 287038)
@@ -119,7 +119,7 @@
if (!axObject.isGroup() && axObject.roleValue() != AccessibilityRole::Div)
return false;
- // Never ignore a <div> with event listeners attached to it (e.g. onclick).
+ // Never ignore a group with event listeners attached to it (e.g. onclick).
if (axObject.node() && axObject.node()->hasEventListeners())
return false;
@@ -127,11 +127,12 @@
if (first && first == axObject.lastChild() && first->roleValue() == AccessibilityRole::StaticText) {
auto childString = first->stringValue();
// stringValue() can be null if the underlying document needs style recalculation.
- if (!childString.isNull()) {
+ if (!childString.isNull() && is<AccessibilityNodeObject>(axObject)) {
Vector<AccessibilityText> axText;
- axObject.accessibilityText(axText);
- // Don't expose <div>s whose only child is text that has the same content as the <div>s accessibility text.
- // Instead, we should expose the text element directly.
+ auto& axNodeObject = downcast<AccessibilityNodeObject>(axObject);
+ axNodeObject.alternativeText(axText);
+ axNodeObject.helpText(axText);
+ // Ignore groups whose accessibility text is the same as their child's static-text content.
auto firstText = axText.size() ? axText[0].text : String();
if (firstText == childString)
return true;
