- Revision
- 284612
- Author
- [email protected]
- Date
- 2021-10-21 09:49:44 -0700 (Thu, 21 Oct 2021)
Log Message
AX: Remove redundant insert of autofill button child in AccessibilityRenderObject::addTextFieldChildren
https://bugs.webkit.org/show_bug.cgi?id=232033
Patch by Tyler Wilcock <[email protected]> on 2021-10-21
Reviewed by Chris Fleizach.
Source/WebCore:
Autofill buttons are represented in the DOM, so there's no reason
to also insert it in AccessibilityRenderObject::addTextFieldChildren.
This results in duplicate objects for this button in the AX tree.
* accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::addTextFieldChildren):
Remove redundant insertion of autofill button element into the
accessibility tree.
LayoutTests:
* accessibility/auto-fill-crash-expected.txt:
* accessibility/auto-fill-crash.html:
* platform/glib/accessibility/auto-fill-crash-expected.txt:
Expect one less child because we no longer insert a redundant button
into the accessibility tree. Also add an expectation confirming the
autofill button is actually a part of the accessibility tree.
* platform/win/accessibility/auto-fill-crash-expected.txt: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (284611 => 284612)
--- trunk/LayoutTests/ChangeLog 2021-10-21 16:49:15 UTC (rev 284611)
+++ trunk/LayoutTests/ChangeLog 2021-10-21 16:49:44 UTC (rev 284612)
@@ -1,3 +1,18 @@
+2021-10-21 Tyler Wilcock <[email protected]>
+
+ AX: Remove redundant insert of autofill button child in AccessibilityRenderObject::addTextFieldChildren
+ https://bugs.webkit.org/show_bug.cgi?id=232033
+
+ Reviewed by Chris Fleizach.
+
+ * accessibility/auto-fill-crash-expected.txt:
+ * accessibility/auto-fill-crash.html:
+ * platform/glib/accessibility/auto-fill-crash-expected.txt:
+ Expect one less child because we no longer insert a redundant button
+ into the accessibility tree. Also add an expectation confirming the
+ autofill button is actually a part of the accessibility tree.
+ * platform/win/accessibility/auto-fill-crash-expected.txt: Added.
+
2021-10-21 Chris Dumez <[email protected]>
Unreviewed, drop test from TestExpectations that no longer exists.
Modified: trunk/LayoutTests/accessibility/auto-fill-crash-expected.txt (284611 => 284612)
--- trunk/LayoutTests/accessibility/auto-fill-crash-expected.txt 2021-10-21 16:49:15 UTC (rev 284611)
+++ trunk/LayoutTests/accessibility/auto-fill-crash-expected.txt 2021-10-21 16:49:44 UTC (rev 284612)
@@ -4,8 +4,9 @@
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
-PASS accessibilityController.accessibleElementById('textfield').childrenCount is 3
-PASS accessibilityController.accessibleElementById('textfield').childrenCount is 1
+PASS textFieldAxObject.childrenCount is 2
+PASS textFieldAxObject.childAtIndex(childrenCountExpected - 1).description is 'AXDescription: contact info AutoFill'
+PASS textFieldAxObject.childrenCount is 1
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/accessibility/auto-fill-crash.html (284611 => 284612)
--- trunk/LayoutTests/accessibility/auto-fill-crash.html 2021-10-21 16:49:15 UTC (rev 284611)
+++ trunk/LayoutTests/accessibility/auto-fill-crash.html 2021-10-21 16:49:44 UTC (rev 284612)
@@ -17,14 +17,24 @@
if (window.accessibilityController) {
var axTextField = accessibilityController.accessibleElementById("textfield");
- var childrenCountExpected = accessibilityController.platformName == "atk" ? "2" : "3";
+ var childrenCountExpected = accessibilityController.platformName == "atk" ? "1" : "2";
window.internals.setShowAutoFillButton(document.getElementById("textfield"), "Contacts");
- shouldBe("accessibilityController.accessibleElementById('textfield').childrenCount", childrenCountExpected);
+ var textFieldAxObject = accessibilityController.accessibleElementById('textfield');
+ shouldBe("textFieldAxObject.childrenCount", childrenCountExpected);
+ // Verify the autofill button is represented in the accessibility tree.
+ shouldBe("textFieldAxObject.childAtIndex(childrenCountExpected - 1).description", "'AXDescription: contact info AutoFill'");
+ var platform = accessibilityController.platformName;
+ // Windows expects 2 children.
+ childrenCountExpected = "2";
+ if (platform == "mac" || platform == "ios")
+ childrenCountExpected = "1"
+ else if (platform == "atk")
+ childrenCountExpected = "0"
+
// Don't crash!
- childrenCountExpected = accessibilityController.platformName == "atk" ? "0" : "1";
window.internals.setShowAutoFillButton(document.getElementById("textfield"), "None");
- shouldBe("accessibilityController.accessibleElementById('textfield').childrenCount", childrenCountExpected);
+ shouldBe("textFieldAxObject.childrenCount", childrenCountExpected);
}
</script>
Modified: trunk/LayoutTests/platform/glib/accessibility/auto-fill-crash-expected.txt (284611 => 284612)
--- trunk/LayoutTests/platform/glib/accessibility/auto-fill-crash-expected.txt 2021-10-21 16:49:15 UTC (rev 284611)
+++ trunk/LayoutTests/platform/glib/accessibility/auto-fill-crash-expected.txt 2021-10-21 16:49:44 UTC (rev 284612)
@@ -4,8 +4,9 @@
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
-PASS accessibilityController.accessibleElementById('textfield').childrenCount is 2
-PASS accessibilityController.accessibleElementById('textfield').childrenCount is 0
+PASS textFieldAxObject.childrenCount is 1
+PASS textFieldAxObject.childAtIndex(childrenCountExpected - 1).description is 'AXDescription: contact info AutoFill'
+PASS textFieldAxObject.childrenCount is 0
PASS successfullyParsed is true
TEST COMPLETE
Copied: trunk/LayoutTests/platform/win/accessibility/auto-fill-crash-expected.txt (from rev 284611, trunk/LayoutTests/accessibility/auto-fill-crash-expected.txt) (0 => 284612)
--- trunk/LayoutTests/platform/win/accessibility/auto-fill-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/platform/win/accessibility/auto-fill-crash-expected.txt 2021-10-21 16:49:44 UTC (rev 284612)
@@ -0,0 +1,13 @@
+
+This tests that when an auto fill element is removed we won't crash accessing an old value.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS textFieldAxObject.childrenCount is 2
+PASS textFieldAxObject.childAtIndex(childrenCountExpected - 1).description is 'AXDescription: contact info AutoFill'
+PASS textFieldAxObject.childrenCount is 2
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Modified: trunk/Source/WebCore/ChangeLog (284611 => 284612)
--- trunk/Source/WebCore/ChangeLog 2021-10-21 16:49:15 UTC (rev 284611)
+++ trunk/Source/WebCore/ChangeLog 2021-10-21 16:49:44 UTC (rev 284612)
@@ -1,5 +1,21 @@
2021-10-21 Tyler Wilcock <[email protected]>
+ AX: Remove redundant insert of autofill button child in AccessibilityRenderObject::addTextFieldChildren
+ https://bugs.webkit.org/show_bug.cgi?id=232033
+
+ Reviewed by Chris Fleizach.
+
+ Autofill buttons are represented in the DOM, so there's no reason
+ to also insert it in AccessibilityRenderObject::addTextFieldChildren.
+ This results in duplicate objects for this button in the AX tree.
+
+ * accessibility/AccessibilityRenderObject.cpp:
+ (WebCore::AccessibilityRenderObject::addTextFieldChildren):
+ Remove redundant insertion of autofill button element into the
+ accessibility tree.
+
+2021-10-21 Tyler Wilcock <[email protected]>
+
AX: Any addition of children should funnel through AccessibilityObject::addChild
https://bugs.webkit.org/show_bug.cgi?id=231914
Modified: trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp (284611 => 284612)
--- trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp 2021-10-21 16:49:15 UTC (rev 284611)
+++ trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp 2021-10-21 16:49:44 UTC (rev 284612)
@@ -3303,13 +3303,7 @@
if (!is<HTMLInputElement>(node))
return;
- HTMLInputElement& input = downcast<HTMLInputElement>(*node);
- if (HTMLElement* autoFillElement = input.autoFillButtonElement()) {
- if (AccessibilityObject* axAutoFill = axObjectCache()->getOrCreate(autoFillElement))
- m_children.append(axAutoFill);
- }
-
- HTMLElement* spinButtonElement = input.innerSpinButtonElement();
+ HTMLElement* spinButtonElement = downcast<HTMLInputElement>(*node).innerSpinButtonElement();
if (!is<SpinButtonElement>(spinButtonElement))
return;
