Changeset 84526 in webkit

Timestamp:

Apr 21, 2011 11:29:00 AM (13 years ago)

Author:

ap@apple.com

Message:

Reviewed by Maciej Stachowiak.

WebKit2: Unconfirmed inline input session persists after reload
​https://bugs.webkit.org/show_bug.cgi?id=59054
<rdar://problem/9291086>

Makes the inline input get properly confirmed or discarded when changing selection,
navigating to another page, focusing another view in the window, or after a web process
crash.

• Shared/EditorState.h: (WebKit::EditorState::EditorState):
• WebProcess/WebPage/WebPage.cpp: (WebKit::WebPage::editorState): Added a variable that Editor sets while processing input method calls. We don't want selection changes caused by -setMarkedText: discard the composition!

• UIProcess/PageClient.h:
• UIProcess/API/mac/PageClientImpl.h:
• UIProcess/API/mac/PageClientImpl.mm: (WebKit::PageClientImpl::updateTextInputState): (WebKit::PageClientImpl::resetTextInputState): Extended updateSecureInputState() to potentially confirm the current inline input session. Added resetTextInputState, which is to be called when we get out of the current web page.

• UIProcess/API/mac/WKView.mm: (-[WKView resignFirstResponder]): Just call _resetTextInputState instead of dealing with input state here. (-[WKView _updateSecureInputState]): Use _isFocused to make the code simpler. (-[WKView _updateTextInputStateIncludingSecureInputState:]): Confirm the current inline input session if selection changed for any other reason but an input method changing the composition. We will want to do other work here, like dismissing a dictionary pop-up, updating a font panel etc. (-[WKView _resetTextInputState]): Added. We don't want secure text input persist after navigations, crashes an such, and we also don't want input methods to remember inline input state. Relying on selection changed notifications is fragile and insufficient.

• UIProcess/API/mac/WKViewInternal.h: Added internal functions to keep the build unbroken.

• UIProcess/WebPageProxy.cpp: (WebKit::WebPageProxy::didCommitLoadForFrame): This is the best chance we have to reset input state for a new page. This doesn't work well for cached page loads, but WK1 is quite broken in that case, too. (WebKit::WebPageProxy::editorStateChanged): Moved some logic up to the client, since the function now does much more than before.

• UIProcess/WebPageProxy.h:
• UIProcess/mac/WebPageProxyMac.mm: (WebKit::WebPageProxy::confirmCompositionWithoutDisturbingSelection):
• WebProcess/WebPage/WebPage.h:
• WebProcess/WebPage/WebPage.messages.in:
• WebProcess/WebPage/mac/WebPageMac.mm: (WebKit::WebPage::confirmCompositionWithoutDisturbingSelection): Added confirmCompositionWithoutDisturbingSelection(). When we confirm the composition due to selection change, we obvisouly don't want the selection to move back.

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• Shared/EditorState.h (modified) (2 diffs)
• UIProcess/API/mac/PageClientImpl.h (modified) (1 diff)
• UIProcess/API/mac/PageClientImpl.mm (modified) (1 diff)
• UIProcess/API/mac/WKView.mm (modified) (3 diffs)
• UIProcess/API/mac/WKViewInternal.h (modified) (2 diffs)
• UIProcess/PageClient.h (modified) (1 diff)
• UIProcess/WebPageProxy.cpp (modified) (2 diffs)
• UIProcess/WebPageProxy.h (modified) (1 diff)
• UIProcess/mac/WebPageProxyMac.mm (modified) (1 diff)
• WebProcess/WebPage/WebPage.cpp (modified) (1 diff)
• WebProcess/WebPage/WebPage.h (modified) (1 diff)
• WebProcess/WebPage/WebPage.messages.in (modified) (1 diff)
• WebProcess/WebPage/mac/WebPageMac.mm (modified) (1 diff)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2011-04-20  Alexey Proskuryakov  <ap@apple.com>
+
+        Reviewed by Maciej Stachowiak.
+
+        WebKit2: Unconfirmed inline input session persists after reload
+        https://bugs.webkit.org/show_bug.cgi?id=59054
+        <rdar://problem/9291086>
+
+        Makes the inline input get properly confirmed or discarded when changing selection,
+        navigating to another page, focusing another view in the window, or after a web process
+        crash.
+
+        * Shared/EditorState.h: (WebKit::EditorState::EditorState):
+        * WebProcess/WebPage/WebPage.cpp: (WebKit::WebPage::editorState):
+        Added a variable that Editor sets while processing input method calls. We don't want
+        selection changes caused by -setMarkedText: discard the composition!
+        
+        * UIProcess/PageClient.h:
+        * UIProcess/API/mac/PageClientImpl.h:
+        * UIProcess/API/mac/PageClientImpl.mm:
+        (WebKit::PageClientImpl::updateTextInputState):
+        (WebKit::PageClientImpl::resetTextInputState):
+        Extended updateSecureInputState() to potentially confirm the current inline input session.
+        Added resetTextInputState, which is to be called when we get out of the current web page.
+
+        * UIProcess/API/mac/WKView.mm:
+        (-[WKView resignFirstResponder]): Just call _resetTextInputState instead of dealing with
+        input state here.
+        (-[WKView _updateSecureInputState]): Use _isFocused to make the code simpler.
+        (-[WKView _updateTextInputStateIncludingSecureInputState:]): Confirm the current inline
+        input session if selection changed for any other reason but an input method changing the
+        composition. We will want to do other work here, like dismissing a dictionary pop-up,
+        updating a font panel etc.
+        (-[WKView _resetTextInputState]): Added. We don't want secure text input persist after
+        navigations, crashes an such, and we also don't want input methods to remember inline input
+        state. Relying on selection changed notifications is fragile and insufficient.
+
+        * UIProcess/API/mac/WKViewInternal.h: Added internal functions to keep the build unbroken.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::didCommitLoadForFrame): This is the best chance we have to reset input
+        state for a new page. This doesn't work well for cached page loads, but WK1 is quite broken
+        in that case, too.
+        (WebKit::WebPageProxy::editorStateChanged): Moved some logic up to the client, since the
+        function now does much more than before.
+
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/mac/WebPageProxyMac.mm: (WebKit::WebPageProxy::confirmCompositionWithoutDisturbingSelection):
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/WebPage.messages.in:
+        * WebProcess/WebPage/mac/WebPageMac.mm: (WebKit::WebPage::confirmCompositionWithoutDisturbingSelection):
+        Added confirmCompositionWithoutDisturbingSelection(). When we confirm the composition due
+        to selection change, we obvisouly don't want the selection to move back.
+
 2011-04-21  Sam Weinig  <sam@webkit.org>
 

trunk/Source/WebKit2/Shared/EditorState.h

@@ -40 +40 @@
         , isInPasswordField(false)
         , hasComposition(false)
+        , shouldIgnoreCompositionSelectionChange(false)
     {
     }
@@ -49 +50 @@
     bool isInPasswordField;
     bool hasComposition;
+    bool shouldIgnoreCompositionSelectionChange;
 };
 

trunk/Source/WebKit2/UIProcess/API/mac/PageClientImpl.h

@@ -75 +75 @@
     virtual bool executeSavedCommandBySelector(const String& selector);
     virtual void setDragImage(const WebCore::IntPoint& clientPosition, PassRefPtr<ShareableBitmap> dragImage, bool isLinkDrag);
-    virtual void updateSecureInputState();
+    virtual void updateTextInputState(bool updateSecureInputState);
+    virtual void resetTextInputState();
 
     virtual WebCore::FloatRect convertToDeviceSpace(const WebCore::FloatRect&);

trunk/Source/WebKit2/UIProcess/API/mac/PageClientImpl.mm

@@ -253 +253 @@
 }
 
-void PageClientImpl::updateSecureInputState()
-{
-    [m_wkView _updateSecureInputState];
+void PageClientImpl::updateTextInputState(bool updateSecureInputState)
+{
+    [m_wkView _updateTextInputStateIncludingSecureInputState:updateSecureInputState];
+}
+
+void PageClientImpl::resetTextInputState()
+{
+    [m_wkView _resetTextInputState];
 }
 

trunk/Source/WebKit2/UIProcess/API/mac/WKView.mm

@@ -311 +311 @@
     _data->_inResignFirstResponder = true;
 
-    if (_data->_inSecureInputState) {
-        DisableSecureEventInput();
-        _data->_inSecureInputState = NO;
-    }
+    [self _resetTextInputState];
+    
     _data->_page->viewStateDidChange(WebPageProxy::ViewIsFocused);
 
@@ -2308 +2306 @@
 - (void)_updateSecureInputState
 {
-    if (![[self window] isKeyWindow] || ([[self window] firstResponder] != self && !_data->_inBecomeFirstResponder)) {
+    if (![[self window] isKeyWindow] || ![self _isFocused]) {
         if (_data->_inSecureInputState) {
             DisableSecureEventInput();
@@ -2332 +2330 @@
 }
 
+- (void)_updateTextInputStateIncludingSecureInputState:(BOOL)updateSecureInputState
+{
+    const EditorState& editorState = _data->_page->editorState();
+    if (updateSecureInputState) {
+        // This is a temporary state when editing. Flipping secure input state too quickly can expose race conditions.
+        if (!editorState.selectionIsNone)
+            [self _updateSecureInputState];
+    }
+
+    if (!editorState.hasComposition || editorState.shouldIgnoreCompositionSelectionChange)
+        return;
+
+    _data->_page->confirmCompositionWithoutDisturbingSelection();
+    [[super inputContext] discardMarkedText];
+}
+
+- (void)_resetTextInputState
+{
+    _data->_page->confirmCompositionWithoutDisturbingSelection();
+    [[super inputContext] discardMarkedText];
+
+    if (_data->_inSecureInputState) {
+        DisableSecureEventInput();
+        _data->_inSecureInputState = NO;
+    }
+}
+
 - (void)_setDrawingAreaSize:(NSSize)size
 {

trunk/Source/WebKit2/UIProcess/API/mac/WKViewInternal.h

@@ -41 +41 @@
     class FindIndicator;
     class LayerTreeContext;
+    struct EditorState;
 }
 
@@ -75 +76 @@
 - (void)_setDragImage:(NSImage *)image at:(NSPoint)clientPoint linkDrag:(BOOL)linkDrag;
 - (void)_updateSecureInputState;
+- (void)_updateTextInputStateIncludingSecureInputState:(BOOL)updateSecureInputState;
+- (void)_resetTextInputState;
 
 - (void)_setDrawingAreaSize:(NSSize)size;

trunk/Source/WebKit2/UIProcess/PageClient.h

@@ -104 +104 @@
     virtual bool executeSavedCommandBySelector(const String& selector) = 0;
     virtual void setDragImage(const WebCore::IntPoint& clientPosition, PassRefPtr<ShareableBitmap> dragImage, bool isLinkDrag) = 0;
-    virtual void updateSecureInputState() = 0;
+    virtual void updateTextInputState(bool updateSecureInputState) = 0;
+    virtual void resetTextInputState() = 0;
 #endif
 #if PLATFORM(WIN)

trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp

@@ -1538 +1538 @@
         return;
 
-#if PLATFORM(MAC) && !defined(BUILDING_ON_SNOW_LEOPARD)
+    WebFrameProxy* frame = process()->webFrame(frameID);
+    MESSAGE_CHECK(frame);
+
+#if PLATFORM(MAC)
+    // FIXME (bug 59111): didCommitLoadForFrame comes too late when restoring a page from b/f cache, making us disable secure event mode in password fields.
+    // FIXME (bug 59121): A load going on in one frame shouldn't affect typing in sibling frames.
+    m_pageClient->resetTextInputState();
+#if !defined(BUILDING_ON_SNOW_LEOPARD)
+    // FIXME: Should this be moved inside resetTextInputState()?
     dismissCorrectionPanel(ReasonForDismissingCorrectionPanelIgnored);
     m_pageClient->dismissDictionaryLookupPanel();
 #endif
-
-    WebFrameProxy* frame = process()->webFrame(frameID);
-    MESSAGE_CHECK(frame);
+#endif
 
     clearLoadDependentCallbacks();
@@ -2152 +2158 @@
 
 #if PLATFORM(MAC)
-    // This is a temporary state. Flipping secure input state too quickly can expose race conditions.
-    if (editorState.selectionIsNone)
-        return;
-
-    if (couldChangeSecureInputState)
-        m_pageClient->updateSecureInputState();
+    m_pageClient->updateTextInputState(couldChangeSecureInputState);
 #endif
 }

trunk/Source/WebKit2/UIProcess/WebPageProxy.h

@@ -259 +259 @@
     void setComposition(const String& text, Vector<WebCore::CompositionUnderline> underlines, uint64_t selectionStart, uint64_t selectionEnd, uint64_t replacementRangeStart, uint64_t replacementRangeEnd);
     void confirmComposition();
+    void confirmCompositionWithoutDisturbingSelection();
     bool insertText(const String& text, uint64_t replacementRangeStart, uint64_t replacementRangeEnd);
     void getMarkedRange(uint64_t& location, uint64_t& length);

trunk/Source/WebKit2/UIProcess/mac/WebPageProxyMac.mm

@@ -152 +152 @@
 }
 
+void WebPageProxy::confirmCompositionWithoutDisturbingSelection()
+{
+    process()->sendSync(Messages::WebPage::ConfirmCompositionWithoutDisturbingSelection(), Messages::WebPage::ConfirmComposition::Reply(m_editorState), m_pageID);
+}
+
 bool WebPageProxy::insertText(const String& text, uint64_t replacementRangeStart, uint64_t replacementRangeEnd)
 {

trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp

@@ -334 +334 @@
     result.isInPasswordField = frame->selection()->isInPasswordField();
     result.hasComposition = frame->editor()->hasComposition();
+    result.shouldIgnoreCompositionSelectionChange = frame->editor()->ignoreCompositionSelectionChange();
     
     return result;

trunk/Source/WebKit2/WebProcess/WebPage/WebPage.h

@@ -326 +326 @@
     void setComposition(const String& text, Vector<WebCore::CompositionUnderline> underlines, uint64_t selectionStart, uint64_t selectionEnd, uint64_t replacementRangeStart, uint64_t replacementRangeEnd, EditorState& newState);
     void confirmComposition(EditorState& newState);
+    void confirmCompositionWithoutDisturbingSelection(EditorState& newState);
     void insertText(const String& text, uint64_t replacementRangeStart, uint64_t replacementRangeEnd, bool& handled, EditorState& newState);
     void getMarkedRange(uint64_t& location, uint64_t& length);

trunk/Source/WebKit2/WebProcess/WebPage/WebPage.messages.in

@@ -188 +188 @@
     SetComposition(WTF::String text, WTF::Vector<WebCore::CompositionUnderline> underlines, uint64_t selectionStart, uint64_t selectionEnd, uint64_t replacementRangeStart, uint64_t replacementRangeEnd) -> (WebKit::EditorState newState)
     ConfirmComposition() -> (WebKit::EditorState newState)
+    ConfirmCompositionWithoutDisturbingSelection() -> (WebKit::EditorState newState)
     InsertText(WTF::String text, uint64_t replacementRangeStart, uint64_t replacementRangeEnd) -> (bool handled, WebKit::EditorState newState)
     GetMarkedRange() -> (uint64_t location, uint64_t length)

trunk/Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm

@@ -244 +244 @@
 
     frame->editor()->confirmComposition();
+
+    newState = editorState();
+}
+
+void WebPage::confirmCompositionWithoutDisturbingSelection(EditorState& newState)
+{
+    Frame* frame = m_page->focusController()->focusedOrMainFrame();
+
+    frame->editor()->confirmCompositionWithoutDisturbingSelection();
 
     newState = editorState();