Title: [177152] trunk/Source
Revision
177152
Author
[email protected]
Date
2014-12-11 09:23:34 -0800 (Thu, 11 Dec 2014)

Log Message

REGRESSION (Async Text Input): Text input method state is not reset when reloading a page
https://bugs.webkit.org/show_bug.cgi?id=139504
rdar://problem/19034674

Reviewed by Enrica Casucci.

Source/WebCore:

Explicitly notify EditorClient when a composition is voluntarily canceled by WebCore.
These are almost certainly not all the places where this happens, but this fixes the bug,
and lays the groundwork for using this new client call instead of didChangeSelection
hacks.

* editing/Editor.cpp:
(WebCore::Editor::clear):
(WebCore::Editor::cancelComposition):
* loader/EmptyClients.h:
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::willTransitionToCommitted):
* page/EditorClient.h:

Source/WebKit/mac:

Stub out new client calls, this patch does not attempt to make any changes on WebKit1.

* WebCoreSupport/WebEditorClient.h:
* WebCoreSupport/WebEditorClient.mm:
(WebEditorClient::discardedComposition):

Source/WebKit/win:

Stub out new client calls, this patch doesn't attempt to make any changes on Windows.

* WebCoreSupport/WebEditorClient.cpp:
(WebEditorClient::discardedComposition):
* WebCoreSupport/WebEditorClient.h:

Source/WebKit2:

WebKit2 used to look at EditorState changes and guess when to cancel a composition.
This was quite unreliable, and needlessly complicated - WebCore knows when it decides
to destroy a composition, so it now explicitly notifies the clients.

* UIProcess/API/mac/WKView.mm: (-[WKView _processDidExit]): Address crashing case too.
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::resetStateAfterProcessExited):
* WebProcess/WebCoreSupport/WebEditorClient.cpp:
(WebKit::WebEditorClient::discardedComposition):
* WebProcess/WebCoreSupport/WebEditorClient.h:
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::didChangeSelection):
(WebKit::WebPage::discardedComposition):
* WebProcess/WebPage/WebPage.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (177151 => 177152)


--- trunk/Source/WebCore/ChangeLog	2014-12-11 17:12:54 UTC (rev 177151)
+++ trunk/Source/WebCore/ChangeLog	2014-12-11 17:23:34 UTC (rev 177152)
@@ -1,3 +1,24 @@
+2014-12-11  Alexey Proskuryakov  <[email protected]>
+
+        REGRESSION (Async Text Input): Text input method state is not reset when reloading a page
+        https://bugs.webkit.org/show_bug.cgi?id=139504
+        rdar://problem/19034674
+
+        Reviewed by Enrica Casucci.
+
+        Explicitly notify EditorClient when a composition is voluntarily canceled by WebCore.
+        These are almost certainly not all the places where this happens, but this fixes the bug,
+        and lays the groundwork for using this new client call instead of didChangeSelection
+        hacks.
+
+        * editing/Editor.cpp:
+        (WebCore::Editor::clear):
+        (WebCore::Editor::cancelComposition):
+        * loader/EmptyClients.h:
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::willTransitionToCommitted):
+        * page/EditorClient.h:
+
 2014-12-10  Anders Carlsson  <[email protected]>
 
         Get rid of the storage strategy

Modified: trunk/Source/WebCore/editing/Editor.cpp (177151 => 177152)


--- trunk/Source/WebCore/editing/Editor.cpp	2014-12-11 17:12:54 UTC (rev 177151)
+++ trunk/Source/WebCore/editing/Editor.cpp	2014-12-11 17:23:34 UTC (rev 177152)
@@ -1091,7 +1091,11 @@
 
 void Editor::clear()
 {
-    m_compositionNode = 0;
+    if (m_compositionNode) {
+        m_compositionNode = nullptr;
+        if (EditorClient* client = this->client())
+            client->discardedComposition(&m_frame);
+    }
     m_customCompositionUnderlines.clear();
     m_shouldStyleWithCSS = false;
     m_defaultParagraphSeparator = EditorParagraphSeparatorIsDiv;

Modified: trunk/Source/WebCore/loader/EmptyClients.h (177151 => 177152)


--- trunk/Source/WebCore/loader/EmptyClients.h	2014-12-11 17:12:54 UTC (rev 177151)
+++ trunk/Source/WebCore/loader/EmptyClients.h	2014-12-11 17:23:34 UTC (rev 177152)
@@ -452,6 +452,7 @@
     virtual void didBeginEditing() override { }
     virtual void respondToChangedContents() override { }
     virtual void respondToChangedSelection(Frame*) override { }
+    virtual void discardedComposition(Frame*) override { }
     virtual void didEndEditing() override { }
     virtual void willWriteSelectionToPasteboard(Range*) override { }
     virtual void didWriteSelectionToPasteboard() override { }

Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (177151 => 177152)


--- trunk/Source/WebCore/loader/FrameLoader.cpp	2014-12-11 17:12:54 UTC (rev 177151)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp	2014-12-11 17:23:34 UTC (rev 177152)
@@ -529,8 +529,10 @@
     if (m_frame.editor().hasComposition()) {
         // The text was already present in DOM, so it's better to confirm than to cancel the composition.
         m_frame.editor().confirmComposition();
-        if (EditorClient* editorClient = m_frame.editor().client())
+        if (EditorClient* editorClient = m_frame.editor().client()) {
             editorClient->respondToChangedSelection(&m_frame);
+            editorClient->discardedComposition(&m_frame);
+        }
     }
 }
 

Modified: trunk/Source/WebCore/page/EditorClient.h (177151 => 177152)


--- trunk/Source/WebCore/page/EditorClient.h	2014-12-11 17:12:54 UTC (rev 177151)
+++ trunk/Source/WebCore/page/EditorClient.h	2014-12-11 17:23:34 UTC (rev 177152)
@@ -97,7 +97,11 @@
     virtual void willWriteSelectionToPasteboard(Range*) = 0;
     virtual void didWriteSelectionToPasteboard() = 0;
     virtual void getClientPasteboardDataForRange(Range*, Vector<String>& pasteboardTypes, Vector<RefPtr<SharedBuffer>>& pasteboardData) = 0;
-    
+
+    // Notify an input method that a composition was voluntarily discarded by WebCore, so that it could clean up too.
+    // This function is not called when a composition is closed per a request from an input method.
+    virtual void discardedComposition(Frame*) = 0;
+
     virtual void registerUndoStep(PassRefPtr<UndoStep>) = 0;
     virtual void registerRedoStep(PassRefPtr<UndoStep>) = 0;
     virtual void clearUndoRedoOperations() = 0;

Modified: trunk/Source/WebKit/mac/ChangeLog (177151 => 177152)


--- trunk/Source/WebKit/mac/ChangeLog	2014-12-11 17:12:54 UTC (rev 177151)
+++ trunk/Source/WebKit/mac/ChangeLog	2014-12-11 17:23:34 UTC (rev 177152)
@@ -1,3 +1,17 @@
+2014-12-11  Alexey Proskuryakov  <[email protected]>
+
+        REGRESSION (Async Text Input): Text input method state is not reset when reloading a page
+        https://bugs.webkit.org/show_bug.cgi?id=139504
+        rdar://problem/19034674
+
+        Reviewed by Enrica Casucci.
+
+        Stub out new client calls, this patch does not attempt to make any changes on WebKit1.
+
+        * WebCoreSupport/WebEditorClient.h:
+        * WebCoreSupport/WebEditorClient.mm:
+        (WebEditorClient::discardedComposition):
+
 2014-12-10  Anders Carlsson  <[email protected]>
 
         Get rid of the storage strategy

Modified: trunk/Source/WebKit/mac/WebCoreSupport/WebEditorClient.h (177151 => 177152)


--- trunk/Source/WebKit/mac/WebCoreSupport/WebEditorClient.h	2014-12-11 17:12:54 UTC (rev 177151)
+++ trunk/Source/WebKit/mac/WebCoreSupport/WebEditorClient.h	2014-12-11 17:23:34 UTC (rev 177152)
@@ -110,6 +110,7 @@
 
     virtual void respondToChangedContents() override;
     virtual void respondToChangedSelection(WebCore::Frame*) override;
+    virtual void discardedComposition(WebCore::Frame*) override;
 
     virtual void registerUndoStep(PassRefPtr<WebCore::UndoStep>) override;
     virtual void registerRedoStep(PassRefPtr<WebCore::UndoStep>) override;

Modified: trunk/Source/WebKit/mac/WebCoreSupport/WebEditorClient.mm (177151 => 177152)


--- trunk/Source/WebKit/mac/WebCoreSupport/WebEditorClient.mm	2014-12-11 17:12:54 UTC (rev 177151)
+++ trunk/Source/WebKit/mac/WebCoreSupport/WebEditorClient.mm	2014-12-11 17:23:34 UTC (rev 177152)
@@ -353,6 +353,11 @@
 #endif
 }
 
+void WebEditorClient::discardedComposition(Frame*)
+{
+    // The effects of this function are currently achieved via -[WebHTMLView _updateSelectionForInputManager].
+}
+
 void WebEditorClient::didEndEditing()
 {
 #if !PLATFORM(IOS)

Modified: trunk/Source/WebKit/win/ChangeLog (177151 => 177152)


--- trunk/Source/WebKit/win/ChangeLog	2014-12-11 17:12:54 UTC (rev 177151)
+++ trunk/Source/WebKit/win/ChangeLog	2014-12-11 17:23:34 UTC (rev 177152)
@@ -1,3 +1,17 @@
+2014-12-11  Alexey Proskuryakov  <[email protected]>
+
+        REGRESSION (Async Text Input): Text input method state is not reset when reloading a page
+        https://bugs.webkit.org/show_bug.cgi?id=139504
+        rdar://problem/19034674
+
+        Reviewed by Enrica Casucci.
+
+        Stub out new client calls, this patch doesn't attempt to make any changes on Windows.
+
+        * WebCoreSupport/WebEditorClient.cpp:
+        (WebEditorClient::discardedComposition):
+        * WebCoreSupport/WebEditorClient.h:
+
 2014-12-10  Anders Carlsson  <[email protected]>
 
         Get rid of the storage strategy

Modified: trunk/Source/WebKit/win/WebCoreSupport/WebEditorClient.cpp (177151 => 177152)


--- trunk/Source/WebKit/win/WebCoreSupport/WebEditorClient.cpp	2014-12-11 17:12:54 UTC (rev 177151)
+++ trunk/Source/WebKit/win/WebCoreSupport/WebEditorClient.cpp	2014-12-11 17:23:34 UTC (rev 177152)
@@ -219,6 +219,11 @@
     notifyCenter->postNotificationName(webViewDidChangeSelectionNotificationName, static_cast<IWebView*>(m_webView), 0);
 }
 
+void WebEditorClient::discardedComposition(Frame*)
+{
+    notImplemented();
+}
+
 void WebEditorClient::didEndEditing()
 {
     notImplemented();

Modified: trunk/Source/WebKit/win/WebCoreSupport/WebEditorClient.h (177151 => 177152)


--- trunk/Source/WebKit/win/WebCoreSupport/WebEditorClient.h	2014-12-11 17:12:54 UTC (rev 177151)
+++ trunk/Source/WebKit/win/WebCoreSupport/WebEditorClient.h	2014-12-11 17:23:34 UTC (rev 177152)
@@ -60,6 +60,7 @@
 
     virtual void respondToChangedContents();
     virtual void respondToChangedSelection(WebCore::Frame*);
+    virtual void discardedComposition(WebCore::Frame*) override;
 
     bool shouldDeleteRange(WebCore::Range*);
 

Modified: trunk/Source/WebKit2/ChangeLog (177151 => 177152)


--- trunk/Source/WebKit2/ChangeLog	2014-12-11 17:12:54 UTC (rev 177151)
+++ trunk/Source/WebKit2/ChangeLog	2014-12-11 17:23:34 UTC (rev 177152)
@@ -1,3 +1,26 @@
+2014-12-11  Alexey Proskuryakov  <[email protected]>
+
+        REGRESSION (Async Text Input): Text input method state is not reset when reloading a page
+        https://bugs.webkit.org/show_bug.cgi?id=139504
+        rdar://problem/19034674
+
+        Reviewed by Enrica Casucci.
+
+        WebKit2 used to look at EditorState changes and guess when to cancel a composition.
+        This was quite unreliable, and needlessly complicated - WebCore knows when it decides
+        to destroy a composition, so it now explicitly notifies the clients.
+
+        * UIProcess/API/mac/WKView.mm: (-[WKView _processDidExit]): Address crashing case too.
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::resetStateAfterProcessExited):
+        * WebProcess/WebCoreSupport/WebEditorClient.cpp:
+        (WebKit::WebEditorClient::discardedComposition):
+        * WebProcess/WebCoreSupport/WebEditorClient.h:
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::didChangeSelection):
+        (WebKit::WebPage::discardedComposition):
+        * WebProcess/WebPage/WebPage.h:
+
 2014-12-10  Anders Carlsson  <[email protected]>
 
         Get rid of the storage strategy

Modified: trunk/Source/WebKit2/UIProcess/API/mac/WKView.mm (177151 => 177152)


--- trunk/Source/WebKit2/UIProcess/API/mac/WKView.mm	2014-12-11 17:12:54 UTC (rev 177151)
+++ trunk/Source/WebKit2/UIProcess/API/mac/WKView.mm	2014-12-11 17:23:34 UTC (rev 177152)
@@ -2891,6 +2891,8 @@
 
 - (void)_processDidExit
 {
+    [self _notifyInputContextAboutDiscardedComposition];
+
     if (_data->_layerHostingView)
         [self _setAcceleratedCompositingModeRootLayer:nil];
 

Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp (177151 => 177152)


--- trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp	2014-12-11 17:12:54 UTC (rev 177151)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp	2014-12-11 17:23:34 UTC (rev 177152)
@@ -4557,6 +4557,11 @@
     m_isValid = false;
     m_isPageSuspended = false;
 
+    m_editorState = EditorState();
+#if PLATFORM(MAC) && !USE(ASYNC_NSTEXTINPUTCLIENT)
+    m_temporarilyClosedComposition = false;
+#endif
+
     m_pageClient.processDidExit();
 
     resetState(ResetStateReason::WebProcessExited);
@@ -4578,12 +4583,6 @@
     m_touchEventQueue.clear();
 #endif
 
-    // FIXME: Reset m_editorState.
-    // FIXME: Notify input methods about abandoned composition.
-#if PLATFORM(MAC) && !USE(ASYNC_NSTEXTINPUTCLIENT)
-    m_temporarilyClosedComposition = false;
-#endif
-
 #if PLATFORM(MAC)
     dismissCorrectionPanel(ReasonForDismissingAlternativeTextIgnored);
     m_pageClient.dismissDictionaryLookupPanel();

Modified: trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.cpp (177151 => 177152)


--- trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.cpp	2014-12-11 17:12:54 UTC (rev 177151)
+++ trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.cpp	2014-12-11 17:23:34 UTC (rev 177152)
@@ -190,6 +190,11 @@
 #endif
 }
 
+void WebEditorClient::discardedComposition(Frame*)
+{
+    m_page->discardedComposition();
+}
+
 void WebEditorClient::didEndEditing()
 {
     static NeverDestroyed<String> WebViewDidEndEditingNotification(ASCIILiteral("WebViewDidEndEditingNotification"));

Modified: trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.h (177151 => 177152)


--- trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.h	2014-12-11 17:12:54 UTC (rev 177151)
+++ trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.h	2014-12-11 17:23:34 UTC (rev 177152)
@@ -64,6 +64,7 @@
     virtual void didBeginEditing() override;
     virtual void respondToChangedContents() override;
     virtual void respondToChangedSelection(WebCore::Frame*) override;
+    virtual void discardedComposition(WebCore::Frame*) override;
     virtual void didEndEditing() override;
     virtual void willWriteSelectionToPasteboard(WebCore::Range*) override;
     virtual void didWriteSelectionToPasteboard() override;

Modified: trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp (177151 => 177152)


--- trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp	2014-12-11 17:12:54 UTC (rev 177151)
+++ trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp	2014-12-11 17:23:34 UTC (rev 177152)
@@ -4379,6 +4379,7 @@
 #if PLATFORM(MAC) && USE(ASYNC_NSTEXTINPUTCLIENT)
     Frame& frame = m_page->focusController().focusedOrMainFrame();
     // Abandon the current inline input session if selection changed for any other reason but an input method direct action.
+    // FIXME: This logic should be in WebCore.
     // FIXME: Many changes that affect composition node do not go through didChangeSelection(). We need to do something when DOM manipulation affects the composition, because otherwise input method's idea about it will be different from Editor's.
     // FIXME: We can't cancel composition when selection changes to NoSelection, but we probably should.
     if (frame.editor().hasComposition() && !frame.editor().ignoreCompositionSelectionChange() && !frame.selection().isNone()) {
@@ -4395,6 +4396,11 @@
 #endif
 }
 
+void WebPage::discardedComposition()
+{
+    send(Messages::WebPageProxy::CompositionWasCanceled(editorState()));
+}
+
 void WebPage::setMinimumLayoutSize(const IntSize& minimumLayoutSize)
 {
     if (m_minimumLayoutSize == minimumLayoutSize)

Modified: trunk/Source/WebKit2/WebProcess/WebPage/WebPage.h (177151 => 177152)


--- trunk/Source/WebKit2/WebProcess/WebPage/WebPage.h	2014-12-11 17:12:54 UTC (rev 177151)
+++ trunk/Source/WebKit2/WebProcess/WebPage/WebPage.h	2014-12-11 17:23:34 UTC (rev 177152)
@@ -600,6 +600,7 @@
 #endif
 
     void didChangeSelection();
+    void discardedComposition();
 
 #if PLATFORM(COCOA)
     void registerUIProcessAccessibilityTokens(const IPC::DataReference& elemenToken, const IPC::DataReference& windowToken);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to