Title: [219638] trunk
- Revision
- 219638
- Author
- [email protected]
- Date
- 2017-07-18 16:08:29 -0700 (Tue, 18 Jul 2017)
Log Message
REGRESSION(r218910): Crash when password field changes to text field
https://bugs.webkit.org/show_bug.cgi?id=174560
Reviewed by Zalan Bujtas.
Source/WebCore:
The crash was caused by textMarkerDataForFirstPositionInTextControl accessing a nullptr returned by getOrCreate.
Unfortunately, in order to this fix bug while preserving the exact behavior would require synchronously creating
a renderer for the editing host when the input type changed since we can't create an accessbility object out of
a renderer-less node.
Instead, revert back to pre-r218910 behavior of always using the text control element's axID when notifying
the value change. While this is inconsistent with the way editing commands report content changes, I've since
learned that VoiceOver has code to deal with this exact situation.
Test: accessibility/mac/input-type-change-crash-2.html
* accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::textMarkerDataForFirstPositionInTextControl):
LayoutTests:
Added a regression test based on the test case provided by Daniel Bates.
* accessibility/mac/input-type-change-crash-2-expected.txt: Added.
* accessibility/mac/input-type-change-crash-2.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (219637 => 219638)
--- trunk/LayoutTests/ChangeLog 2017-07-18 22:54:00 UTC (rev 219637)
+++ trunk/LayoutTests/ChangeLog 2017-07-18 23:08:29 UTC (rev 219638)
@@ -1,3 +1,15 @@
+2017-07-18 Ryosuke Niwa <[email protected]>
+
+ REGRESSION(r218910): Crash when password field changes to text field
+ https://bugs.webkit.org/show_bug.cgi?id=174560
+
+ Reviewed by Zalan Bujtas.
+
+ Added a regression test based on the test case provided by Daniel Bates.
+
+ * accessibility/mac/input-type-change-crash-2-expected.txt: Added.
+ * accessibility/mac/input-type-change-crash-2.html: Added.
+
2017-07-18 Matt Baker <[email protected]>
Web Inspector: Refactoring: replace InspectorCanvasAgent::CanvasEntry with a helper class
Added: trunk/LayoutTests/accessibility/mac/input-type-change-crash-2-expected.txt (0 => 219638)
--- trunk/LayoutTests/accessibility/mac/input-type-change-crash-2-expected.txt (rev 0)
+++ trunk/LayoutTests/accessibility/mac/input-type-change-crash-2-expected.txt 2017-07-18 23:08:29 UTC (rev 219638)
@@ -0,0 +1,3 @@
+This tests changing the password field's type to text while the accessibility tree is turned on. WebKit should not crash.
+
+PASS - WebKit did not crash.
Added: trunk/LayoutTests/accessibility/mac/input-type-change-crash-2.html (0 => 219638)
--- trunk/LayoutTests/accessibility/mac/input-type-change-crash-2.html (rev 0)
+++ trunk/LayoutTests/accessibility/mac/input-type-change-crash-2.html 2017-07-18 23:08:29 UTC (rev 219638)
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>This tests changing the password field's type to text while the accessibility tree is turned on. WebKit should not crash.</p>
+<input type="password" value="hello" _onmouseup_="this.setAttribute('type', 'text')">
+<script>
+if (!window.accessibilityController) {
+ document.write('Click the text input on the left. WebKit should not crash.');
+} else {
+ testRunner.dumpAsText();
+ accessibilityController.enableEnhancedAccessibility(true);
+ internals.updateLayoutIgnorePendingStylesheetsAndRunPostLayoutTasks();
+ const input = document.querySelector('input');
+ input.type = 'text';
+ input.remove();
+ document.write('PASS - WebKit did not crash.');
+}
+
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (219637 => 219638)
--- trunk/Source/WebCore/ChangeLog 2017-07-18 22:54:00 UTC (rev 219637)
+++ trunk/Source/WebCore/ChangeLog 2017-07-18 23:08:29 UTC (rev 219638)
@@ -1,3 +1,24 @@
+2017-07-18 Ryosuke Niwa <[email protected]>
+
+ REGRESSION(r218910): Crash when password field changes to text field
+ https://bugs.webkit.org/show_bug.cgi?id=174560
+
+ Reviewed by Zalan Bujtas.
+
+ The crash was caused by textMarkerDataForFirstPositionInTextControl accessing a nullptr returned by getOrCreate.
+ Unfortunately, in order to this fix bug while preserving the exact behavior would require synchronously creating
+ a renderer for the editing host when the input type changed since we can't create an accessbility object out of
+ a renderer-less node.
+
+ Instead, revert back to pre-r218910 behavior of always using the text control element's axID when notifying
+ the value change. While this is inconsistent with the way editing commands report content changes, I've since
+ learned that VoiceOver has code to deal with this exact situation.
+
+ Test: accessibility/mac/input-type-change-crash-2.html
+
+ * accessibility/AXObjectCache.cpp:
+ (WebCore::AXObjectCache::textMarkerDataForFirstPositionInTextControl):
+
2017-07-18 Matt Baker <[email protected]>
Web Inspector: Refactoring: replace InspectorCanvasAgent::CanvasEntry with a helper class
Modified: trunk/Source/WebCore/accessibility/AXObjectCache.cpp (219637 => 219638)
--- trunk/Source/WebCore/accessibility/AXObjectCache.cpp 2017-07-18 22:54:00 UTC (rev 219637)
+++ trunk/Source/WebCore/accessibility/AXObjectCache.cpp 2017-07-18 23:08:29 UTC (rev 219638)
@@ -2177,27 +2177,17 @@
// This function exits as a performance optimization to avoid a synchronous layout.
std::optional<TextMarkerData> AXObjectCache::textMarkerDataForFirstPositionInTextControl(HTMLTextFormControlElement& textControl)
{
- TextControlInnerTextElement* innerTextElement = textControl.innerTextElement();
- if (!innerTextElement)
- return std::nullopt;
-
if (is<HTMLInputElement>(textControl) && downcast<HTMLInputElement>(textControl).isPasswordField())
return std::nullopt;
- Position firstPosition = firstPositionInNode(innerTextElement);
- Node* firstChild = innerTextElement->firstChild();
- if (!firstChild)
- firstChild = innerTextElement;
- ContainerNode* editingHost = highestEditableRoot(firstPosition);
- if (!editingHost) // textControl is no longer editable. e.g. readonly or disabled.
+ AXObjectCache* cache = textControl.document().axObjectCache();
+ RefPtr<AccessibilityObject> obj = cache->getOrCreate(&textControl);
+ if (!obj)
return std::nullopt;
- AXObjectCache* cache = textControl.document().axObjectCache();
- RefPtr<AccessibilityObject> obj = cache->getOrCreate(editingHost);
-
TextMarkerData textMarkerData;
textMarkerData.axID = obj.get()->axObjectID();
- textMarkerData.node = firstChild;
+ textMarkerData.node = &textControl;
cache->setNodeInUse(&textControl);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
