REGRESSION (Async Text Input): Text input method state is not reset when reloading...
authorap@apple.com <ap@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Dec 2014 17:23:34 +0000 (17:23 +0000)
committerap@apple.com <ap@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Dec 2014 17:23:34 +0000 (17:23 +0000)
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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@177152 268f45cc-cd09-0410-ab3c-d52691b4dbfc

18 files changed:
Source/WebCore/ChangeLog
Source/WebCore/editing/Editor.cpp
Source/WebCore/loader/EmptyClients.h
Source/WebCore/loader/FrameLoader.cpp
Source/WebCore/page/EditorClient.h
Source/WebKit/mac/ChangeLog
Source/WebKit/mac/WebCoreSupport/WebEditorClient.h
Source/WebKit/mac/WebCoreSupport/WebEditorClient.mm
Source/WebKit/win/ChangeLog
Source/WebKit/win/WebCoreSupport/WebEditorClient.cpp
Source/WebKit/win/WebCoreSupport/WebEditorClient.h
Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/API/mac/WKView.mm
Source/WebKit2/UIProcess/WebPageProxy.cpp
Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.cpp
Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.h
Source/WebKit2/WebProcess/WebPage/WebPage.cpp
Source/WebKit2/WebProcess/WebPage/WebPage.h

index f6a43e1..b874359 100644 (file)
@@ -1,3 +1,24 @@
+2014-12-11  Alexey Proskuryakov  <ap@apple.com>
+
+        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  <andersca@apple.com>
 
         Get rid of the storage strategy
index 4ba9295..5d73417 100644 (file)
@@ -1091,7 +1091,11 @@ Editor::~Editor()
 
 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;
index 8c7a432..57971c5 100644 (file)
@@ -452,6 +452,7 @@ public:
     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 { }
index d5b4cf2..5e57e95 100644 (file)
@@ -529,8 +529,10 @@ void FrameLoader::willTransitionToCommitted()
     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);
+        }
     }
 }
 
index 66c3e66..e18c027 100644 (file)
@@ -97,7 +97,11 @@ public:
     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;
index 1bd072d..bb4c02a 100644 (file)
@@ -1,3 +1,17 @@
+2014-12-11  Alexey Proskuryakov  <ap@apple.com>
+
+        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  <andersca@apple.com>
 
         Get rid of the storage strategy
index 1067df2..90e166a 100644 (file)
@@ -110,6 +110,7 @@ private:
 
     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;
index ec7180f..0b8022b 100644 (file)
@@ -353,6 +353,11 @@ void WebEditorClient::respondToChangedSelection(Frame* frame)
 #endif
 }
 
+void WebEditorClient::discardedComposition(Frame*)
+{
+    // The effects of this function are currently achieved via -[WebHTMLView _updateSelectionForInputManager].
+}
+
 void WebEditorClient::didEndEditing()
 {
 #if !PLATFORM(IOS)
index 10a87a2..337cbdc 100644 (file)
@@ -1,3 +1,17 @@
+2014-12-11  Alexey Proskuryakov  <ap@apple.com>
+
+        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  <andersca@apple.com>
 
         Get rid of the storage strategy
index 8ab5795..6a8f6f2 100644 (file)
@@ -219,6 +219,11 @@ void WebEditorClient::respondToChangedSelection(Frame*)
     notifyCenter->postNotificationName(webViewDidChangeSelectionNotificationName, static_cast<IWebView*>(m_webView), 0);
 }
 
+void WebEditorClient::discardedComposition(Frame*)
+{
+    notImplemented();
+}
+
 void WebEditorClient::didEndEditing()
 {
     notImplemented();
index 1bad32d..4813761 100644 (file)
@@ -60,6 +60,7 @@ public:
 
     virtual void respondToChangedContents();
     virtual void respondToChangedSelection(WebCore::Frame*);
+    virtual void discardedComposition(WebCore::Frame*) override;
 
     bool shouldDeleteRange(WebCore::Range*);
 
index 3d1ac76..3b38d7f 100644 (file)
@@ -1,3 +1,26 @@
+2014-12-11  Alexey Proskuryakov  <ap@apple.com>
+
+        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  <andersca@apple.com>
 
         Get rid of the storage strategy
index 787d638..87314e5 100644 (file)
@@ -2891,6 +2891,8 @@ static void* keyValueObservingContext = &keyValueObservingContext;
 
 - (void)_processDidExit
 {
+    [self _notifyInputContextAboutDiscardedComposition];
+
     if (_data->_layerHostingView)
         [self _setAcceleratedCompositingModeRootLayer:nil];
 
index c3ca07b..86d44de 100644 (file)
@@ -4557,6 +4557,11 @@ void WebPageProxy::resetStateAfterProcessExited()
     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 @@ void WebPageProxy::resetStateAfterProcessExited()
     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();
index 36b54af..15fdd6f 100644 (file)
@@ -190,6 +190,11 @@ void WebEditorClient::respondToChangedSelection(Frame* frame)
 #endif
 }
 
+void WebEditorClient::discardedComposition(Frame*)
+{
+    m_page->discardedComposition();
+}
+
 void WebEditorClient::didEndEditing()
 {
     static NeverDestroyed<String> WebViewDidEndEditingNotification(ASCIILiteral("WebViewDidEndEditingNotification"));
index 6a68f2d..5fc6131 100644 (file)
@@ -64,6 +64,7 @@ private:
     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;
index e0c992d..49a9c34 100644 (file)
@@ -4379,6 +4379,7 @@ void WebPage::didChangeSelection()
 #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 @@ void WebPage::didChangeSelection()
 #endif
 }
 
+void WebPage::discardedComposition()
+{
+    send(Messages::WebPageProxy::CompositionWasCanceled(editorState()));
+}
+
 void WebPage::setMinimumLayoutSize(const IntSize& minimumLayoutSize)
 {
     if (m_minimumLayoutSize == minimumLayoutSize)
index e1410d9..2d20b15 100644 (file)
@@ -600,6 +600,7 @@ public:
 #endif
 
     void didChangeSelection();
+    void discardedComposition();
 
 #if PLATFORM(COCOA)
     void registerUIProcessAccessibilityTokens(const IPC::DataReference& elemenToken, const IPC::DataReference& windowToken);