Title: [252062] trunk
Revision
252062
Author
[email protected]
Date
2019-11-05 10:52:12 -0800 (Tue, 05 Nov 2019)

Log Message

Native text substitutions interfere with HTML <datalist> options resulting in crash
https://bugs.webkit.org/show_bug.cgi?id=203116
<rdar://problem/49875932>

Reviewed by Tim Horton.

Source/WebKit:

On macOS, an NSTableView inside a separate window is used to render suggestions when showing a datalist. The
crash happens when this table view is reloaded while clicking a datalist suggestion; that is, if -[NSTableView
reloadData] is invoked after the user sends a platform MouseDown event on the table view cell but before the
MouseUp is received, the selected row of the table view will be -1 when the action, `-selectedRow:`, is invoked.

In this particular case, the table view reload is triggered as a result of hiding the autocorrection bubble on
macOS, thereby committing the alternative text suggestion and changing the value of the text field via an
editing command.

To avoid crashing, we simply make `-selectedRow:` robust in the case where the index is invalid.

Test: fast/forms/datalist/datalist-click-crash.html

* UIProcess/mac/WebDataListSuggestionsDropdownMac.mm:
(-[WKDataListSuggestionsController selectedRow:]):

Tools:

Add a new testing hook to wait for datalist suggestions to show up and choose the suggestion at the given index.

* DumpRenderTree/mac/UIScriptControllerMac.h:
* DumpRenderTree/mac/UIScriptControllerMac.mm:
(WTR::UIScriptControllerMac::activateDataListSuggestion):
* TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
* TestRunnerShared/UIScriptContext/UIScriptController.h:
(WTR::UIScriptController::activateDataListSuggestion):
* WebKitTestRunner/ios/UIScriptControllerIOS.h:
* WebKitTestRunner/ios/UIScriptControllerIOS.mm:
(WTR::UIScriptControllerIOS::activateDataListSuggestion):
* WebKitTestRunner/mac/UIScriptControllerMac.h:
* WebKitTestRunner/mac/UIScriptControllerMac.mm:
(WTR::UIScriptControllerMac::isShowingDataListSuggestions const):
(WTR::UIScriptControllerMac::activateDataListSuggestion):

Dig through the view hierarchy of the NSWindow subclass used to show datalist suggestions for the table view
containing the suggestions; then, select the given row, and invoke the action on the target.

(WTR::UIScriptControllerMac::dataListSuggestionsTableView const):

LayoutTests:

Add a new layout test to exercise the crash.

* fast/forms/datalist/datalist-click-crash-expected.txt: Added.
* fast/forms/datalist/datalist-click-crash.html: Added.
* resources/ui-helper.js:
(window.UIHelper.activateDataListSuggestion):

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (252061 => 252062)


--- trunk/LayoutTests/ChangeLog	2019-11-05 18:39:49 UTC (rev 252061)
+++ trunk/LayoutTests/ChangeLog	2019-11-05 18:52:12 UTC (rev 252062)
@@ -1,3 +1,18 @@
+2019-11-05  Wenson Hsieh  <[email protected]>
+
+        Native text substitutions interfere with HTML <datalist> options resulting in crash
+        https://bugs.webkit.org/show_bug.cgi?id=203116
+        <rdar://problem/49875932>
+
+        Reviewed by Tim Horton.
+
+        Add a new layout test to exercise the crash.
+
+        * fast/forms/datalist/datalist-click-crash-expected.txt: Added.
+        * fast/forms/datalist/datalist-click-crash.html: Added.
+        * resources/ui-helper.js:
+        (window.UIHelper.activateDataListSuggestion):
+
 2019-11-05  Andy Estes  <[email protected]>
 
         ApplePaySession should never prevent entering the back/forward cache

Added: trunk/LayoutTests/fast/forms/datalist/datalist-click-crash-expected.txt (0 => 252062)


--- trunk/LayoutTests/fast/forms/datalist/datalist-click-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/forms/datalist/datalist-click-crash-expected.txt	2019-11-05 18:52:12 UTC (rev 252062)
@@ -0,0 +1,11 @@
+This test verifies that we don't crash when selecting a datalist suggestion, if the suggestions list is reloaded during the click (after mousedown and before mouseup). To manually run the test, click 'Foo' in the datalist dropdown. The application should not crash.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Did not crash.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
+

Added: trunk/LayoutTests/fast/forms/datalist/datalist-click-crash.html (0 => 252062)


--- trunk/LayoutTests/fast/forms/datalist/datalist-click-crash.html	                        (rev 0)
+++ trunk/LayoutTests/fast/forms/datalist/datalist-click-crash.html	2019-11-05 18:52:12 UTC (rev 252062)
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <script src=""
+    <script src=""
+</head>
+<body>
+<input list="foobar" />
+<datalist id="foobar">
+    <select>
+        <option>Foo</option>
+    </select>
+</datalist>
+<br>
+</body>
+<script>
+jsTestIsAsync = true;
+
+addEventListener("load", async function() {
+    description("This test verifies that we don't crash when selecting a datalist suggestion, if the suggestions list is reloaded during the click (after mousedown and before mouseup). To manually run the test, click 'Foo' in the datalist dropdown. The application should not crash.");
+    if (!window.testRunner)
+        return;
+
+    await UIHelper.activateDataListSuggestion(0);
+    document.querySelectorAll("input, select").forEach(element => element.remove());
+    testPassed("Did not crash.");
+    finishJSTest();
+});
+
+(async function() {
+    while (true) {
+        const input = document.querySelector("input");
+        if (!input)
+            return;
+
+        input.focus();
+        document.execCommand("InsertText", true, "");
+        await new Promise(requestAnimationFrame);
+    }
+})();
+</script>
+</html>

Modified: trunk/LayoutTests/resources/ui-helper.js (252061 => 252062)


--- trunk/LayoutTests/resources/ui-helper.js	2019-11-05 18:39:49 UTC (rev 252061)
+++ trunk/LayoutTests/resources/ui-helper.js	2019-11-05 18:52:12 UTC (rev 252062)
@@ -666,6 +666,13 @@
         });
     }
 
+    static activateDataListSuggestion(index) {
+        const script = `uiController.activateDataListSuggestion(${index}, () => {
+            uiController.uiScriptComplete("");
+        });`;
+        return new Promise(resolve => testRunner.runUIScript(script, resolve));
+    }
+
     static isShowingDataListSuggestions()
     {
         return new Promise(resolve => {

Modified: trunk/Source/WebKit/ChangeLog (252061 => 252062)


--- trunk/Source/WebKit/ChangeLog	2019-11-05 18:39:49 UTC (rev 252061)
+++ trunk/Source/WebKit/ChangeLog	2019-11-05 18:52:12 UTC (rev 252062)
@@ -1,3 +1,27 @@
+2019-11-05  Wenson Hsieh  <[email protected]>
+
+        Native text substitutions interfere with HTML <datalist> options resulting in crash
+        https://bugs.webkit.org/show_bug.cgi?id=203116
+        <rdar://problem/49875932>
+
+        Reviewed by Tim Horton.
+
+        On macOS, an NSTableView inside a separate window is used to render suggestions when showing a datalist. The
+        crash happens when this table view is reloaded while clicking a datalist suggestion; that is, if -[NSTableView
+        reloadData] is invoked after the user sends a platform MouseDown event on the table view cell but before the
+        MouseUp is received, the selected row of the table view will be -1 when the action, `-selectedRow:`, is invoked.
+
+        In this particular case, the table view reload is triggered as a result of hiding the autocorrection bubble on
+        macOS, thereby committing the alternative text suggestion and changing the value of the text field via an
+        editing command.
+
+        To avoid crashing, we simply make `-selectedRow:` robust in the case where the index is invalid.
+
+        Test: fast/forms/datalist/datalist-click-crash.html
+
+        * UIProcess/mac/WebDataListSuggestionsDropdownMac.mm:
+        (-[WKDataListSuggestionsController selectedRow:]):
+
 2019-11-05  Sihui Liu  <[email protected]>
 
         REGRESSION (r250754): web page using IDBIndex doesn't load occasionally

Modified: trunk/Source/WebKit/UIProcess/mac/WebDataListSuggestionsDropdownMac.mm (252061 => 252062)


--- trunk/Source/WebKit/UIProcess/mac/WebDataListSuggestionsDropdownMac.mm	2019-11-05 18:39:49 UTC (rev 252061)
+++ trunk/Source/WebKit/UIProcess/mac/WebDataListSuggestionsDropdownMac.mm	2019-11-05 18:52:12 UTC (rev 252062)
@@ -402,7 +402,11 @@
 
 - (void)selectedRow:(NSTableView *)sender
 {
-    _dropdown->didSelectOption(_suggestions.at([sender selectedRow]));
+    auto selectedString = self.currentSelectedString;
+    if (!selectedString)
+        return;
+
+    _dropdown->didSelectOption(selectedString);
 }
 
 - (NSInteger)numberOfRowsInTableView:(NSTableView *)tableView

Modified: trunk/Tools/ChangeLog (252061 => 252062)


--- trunk/Tools/ChangeLog	2019-11-05 18:39:49 UTC (rev 252061)
+++ trunk/Tools/ChangeLog	2019-11-05 18:52:12 UTC (rev 252062)
@@ -1,3 +1,32 @@
+2019-11-05  Wenson Hsieh  <[email protected]>
+
+        Native text substitutions interfere with HTML <datalist> options resulting in crash
+        https://bugs.webkit.org/show_bug.cgi?id=203116
+        <rdar://problem/49875932>
+
+        Reviewed by Tim Horton.
+
+        Add a new testing hook to wait for datalist suggestions to show up and choose the suggestion at the given index.
+
+        * DumpRenderTree/mac/UIScriptControllerMac.h:
+        * DumpRenderTree/mac/UIScriptControllerMac.mm:
+        (WTR::UIScriptControllerMac::activateDataListSuggestion):
+        * TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
+        * TestRunnerShared/UIScriptContext/UIScriptController.h:
+        (WTR::UIScriptController::activateDataListSuggestion):
+        * WebKitTestRunner/ios/UIScriptControllerIOS.h:
+        * WebKitTestRunner/ios/UIScriptControllerIOS.mm:
+        (WTR::UIScriptControllerIOS::activateDataListSuggestion):
+        * WebKitTestRunner/mac/UIScriptControllerMac.h:
+        * WebKitTestRunner/mac/UIScriptControllerMac.mm:
+        (WTR::UIScriptControllerMac::isShowingDataListSuggestions const):
+        (WTR::UIScriptControllerMac::activateDataListSuggestion):
+
+        Dig through the view hierarchy of the NSWindow subclass used to show datalist suggestions for the table view
+        containing the suggestions; then, select the given row, and invoke the action on the target.
+
+        (WTR::UIScriptControllerMac::dataListSuggestionsTableView const):
+
 2019-11-05  Daniel Bates  <[email protected]>
 
         TestWebKitAPI.WebKit.DocumentEditingContext is failing on iPad

Modified: trunk/Tools/DumpRenderTree/mac/UIScriptControllerMac.h (252061 => 252062)


--- trunk/Tools/DumpRenderTree/mac/UIScriptControllerMac.h	2019-11-05 18:39:49 UTC (rev 252061)
+++ trunk/Tools/DumpRenderTree/mac/UIScriptControllerMac.h	2019-11-05 18:52:12 UTC (rev 252062)
@@ -50,6 +50,7 @@
     void simulateAccessibilitySettingsChangeNotification(JSValueRef) override;
     NSUndoManager *platformUndoManager() const override;
     void copyText(JSStringRef) override;
+    void activateDataListSuggestion(long, JSValueRef) override;
 };
 
 }

Modified: trunk/Tools/DumpRenderTree/mac/UIScriptControllerMac.mm (252061 => 252062)


--- trunk/Tools/DumpRenderTree/mac/UIScriptControllerMac.mm	2019-11-05 18:39:49 UTC (rev 252061)
+++ trunk/Tools/DumpRenderTree/mac/UIScriptControllerMac.mm	2019-11-05 18:52:12 UTC (rev 252062)
@@ -100,6 +100,19 @@
 #endif
 }
 
+void UIScriptControllerMac::activateDataListSuggestion(long index, JSValueRef callback)
+{
+    // FIXME: Not implemented.
+    UNUSED_PARAM(index);
+
+    unsigned callbackID = m_context->prepareForAsyncTask(callback, CallbackTypeNonPersistent);
+    dispatch_async(dispatch_get_main_queue(), ^{
+        if (!m_context)
+            return;
+        m_context->asyncTaskComplete(callbackID);
+    });
+}
+
 void UIScriptControllerMac::overridePreference(JSStringRef preferenceRef, JSStringRef valueRef)
 {
     WebPreferences *preferences = mainFrame.webView.preferences;

Modified: trunk/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl (252061 => 252062)


--- trunk/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl	2019-11-05 18:39:49 UTC (rev 252061)
+++ trunk/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl	2019-11-05 18:52:12 UTC (rev 252062)
@@ -219,6 +219,7 @@
 
     // <datalist>
     readonly attribute boolean isShowingDataListSuggestions;
+    void activateDataListSuggestion(unsigned long index, object callback);
 
     void keyboardAccessoryBarNext();
     void keyboardAccessoryBarPrevious();

Modified: trunk/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h (252061 => 252062)


--- trunk/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h	2019-11-05 18:39:49 UTC (rev 252061)
+++ trunk/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h	2019-11-05 18:52:12 UTC (rev 252062)
@@ -201,6 +201,7 @@
     virtual JSObjectRef calendarType() const { notImplemented(); return nullptr; }
     virtual void setDefaultCalendarType(JSStringRef calendarIdentifier) { notImplemented(); }
     virtual JSObjectRef inputViewBounds() const { notImplemented(); return nullptr; }
+    virtual void activateDataListSuggestion(long, JSValueRef) { notImplemented(); }
 
     // Share Sheet
 

Modified: trunk/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.h (252061 => 252062)


--- trunk/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.h	2019-11-05 18:39:49 UTC (rev 252061)
+++ trunk/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.h	2019-11-05 18:52:12 UTC (rev 252062)
@@ -129,6 +129,7 @@
     void beginBackSwipe(JSValueRef) override;
     void completeBackSwipe(JSValueRef) override;
     bool isShowingDataListSuggestions() const override;
+    void activateDataListSuggestion(long, JSValueRef) override;
     void drawSquareInEditableImage() override;
     long numberOfStrokesInEditableImage() override;
     void setKeyboardInputModeIdentifier(JSStringRef) override;

Modified: trunk/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm (252061 => 252062)


--- trunk/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm	2019-11-05 18:39:49 UTC (rev 252061)
+++ trunk/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm	2019-11-05 18:52:12 UTC (rev 252062)
@@ -1032,6 +1032,19 @@
     [webView() _completeBackSwipeForTesting];
 }
 
+void UIScriptControllerIOS::activateDataListSuggestion(long index, JSValueRef callback)
+{
+    // FIXME: Not implemented.
+    UNUSED_PARAM(index);
+
+    unsigned callbackID = m_context->prepareForAsyncTask(callback, CallbackTypeNonPersistent);
+    dispatch_async(dispatch_get_main_queue(), ^{
+        if (!m_context)
+            return;
+        m_context->asyncTaskComplete(callbackID);
+    });
+}
+
 bool UIScriptControllerIOS::isShowingDataListSuggestions() const
 {
     Class remoteKeyboardWindowClass = NSClassFromString(@"UIRemoteKeyboardWindow");

Modified: trunk/Tools/WebKitTestRunner/mac/UIScriptControllerMac.h (252061 => 252062)


--- trunk/Tools/WebKitTestRunner/mac/UIScriptControllerMac.h	2019-11-05 18:39:49 UTC (rev 252061)
+++ trunk/Tools/WebKitTestRunner/mac/UIScriptControllerMac.h	2019-11-05 18:52:12 UTC (rev 252062)
@@ -43,6 +43,7 @@
     double zoomScale() const override;
     void simulateAccessibilitySettingsChangeNotification(JSValueRef) override;
     bool isShowingDataListSuggestions() const override;
+    void activateDataListSuggestion(long index, JSValueRef callback) override;
     void beginBackSwipe(JSValueRef) override;
     void completeBackSwipe(JSValueRef) override;
     void playBackEventStream(JSStringRef, JSValueRef) override;
@@ -57,6 +58,9 @@
     void chooseMenuAction(JSStringRef, JSValueRef) override;
 
     void activateAtPoint(long x, long y, JSValueRef callback) override;
+
+private:
+    NSTableView *dataListSuggestionsTableView() const;
 };
 
 } // namespace WTR

Modified: trunk/Tools/WebKitTestRunner/mac/UIScriptControllerMac.mm (252061 => 252062)


--- trunk/Tools/WebKitTestRunner/mac/UIScriptControllerMac.mm	2019-11-05 18:39:49 UTC (rev 252061)
+++ trunk/Tools/WebKitTestRunner/mac/UIScriptControllerMac.mm	2019-11-05 18:52:12 UTC (rev 252062)
@@ -28,6 +28,7 @@
 
 #import "EventSenderProxy.h"
 #import "EventSerializerMac.h"
+#import "PlatformViewHelpers.h"
 #import "PlatformWebView.h"
 #import "SharedEventStreamsMac.h"
 #import "TestController.h"
@@ -96,11 +97,38 @@
 
 bool UIScriptControllerMac::isShowingDataListSuggestions() const
 {
+    return dataListSuggestionsTableView();
+}
+
+void UIScriptControllerMac::activateDataListSuggestion(long index, JSValueRef callback)
+{
+    unsigned callbackID = m_context->prepareForAsyncTask(callback, CallbackTypeNonPersistent);
+
+    RetainPtr<NSTableView> table;
+    do {
+        table = dataListSuggestionsTableView();
+    } while (index >= [table numberOfRows] && [[NSRunLoop currentRunLoop] runMode:NSDefaultRunLoopMode beforeDate:[NSDate distantPast]]);
+
+    [table selectRowIndexes:[NSIndexSet indexSetWithIndex:index] byExtendingSelection:NO];
+
+    // Send the action after a short delay to simulate normal user interaction.
+    dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(0.05 * NSEC_PER_SEC)), dispatch_get_main_queue(), [this, protectedThis = makeRefPtr(*this), callbackID, table] {
+        if ([table window])
+            [table sendAction:[table action] to:[table target]];
+
+        if (!m_context)
+            return;
+        m_context->asyncTaskComplete(callbackID);
+    });
+}
+
+NSTableView *UIScriptControllerMac::dataListSuggestionsTableView() const
+{
     for (NSWindow *childWindow in webView().window.childWindows) {
         if ([childWindow isKindOfClass:NSClassFromString(@"WKDataListSuggestionWindow")])
-            return true;
+            return (NSTableView *)[findAllViewsInHierarchyOfType(childWindow.contentView, NSClassFromString(@"WKDataListSuggestionTableView")) firstObject];
     }
-    return false;
+    return nil;
 }
 
 static void playBackEvents(WKWebView *webView, UIScriptContext *context, NSString *eventStream, JSValueRef callback)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to