diff --git a/dom/ipc/PBrowser.ipdl b/dom/ipc/PBrowser.ipdl index c0002890860..a76668ef62e 100644 --- a/dom/ipc/PBrowser.ipdl +++ b/dom/ipc/PBrowser.ipdl @@ -91,6 +91,26 @@ parent: sync SyncMessage(nsString aMessage, nsString aJSON) returns (nsString[] retval); + /** + * The IME sequence number (seqno) parameter is used to make sure + * that a notification is discarded if it arrives at the chrome process + * too late. If the notification is late and we accept it, we will have + * an out-of-date view of the content process, which means events that we + * dispatch based on this out-of-date view will be wrong also. + * (see Bug 599550 and Bug 591047 comments 44, 50, and 54) + * + * Chrome increments seqno and includes it in each IME event sent to + * content, and content sends its current seqno back to chrome with each + * notification. A notification is up-to-date only if the content + * seqno is the same as the current chrome seqno, meaning no additional + * event was sent to content before the notification was received + * + * On blur, chrome returns the current seqno to content, and content + * uses it to discard subsequent events until the content seqno and + * chrome seqno-on-blur match again. These events, meant for the blurred + * textfield, are discarded to prevent events going to the wrong target + */ + /** * Notifies chrome that there is a focus change involving an editable * object (input, textarea, document, contentEditable. etc.) @@ -98,9 +118,10 @@ parent: * focus PR_TRUE if editable object is receiving focus * PR_FALSE if losing focus * preference Native widget preference for IME updates + * seqno Current seqno value on the chrome side */ sync NotifyIMEFocus(PRBool focus) - returns (nsIMEUpdatePreference preference); + returns (nsIMEUpdatePreference preference, PRUint32 seqno); /** * Notifies chrome that there has been a change in text content @@ -120,10 +141,11 @@ parent: * Notifies chrome that there has been a change in selection * Only called when NotifyIMEFocus returns PR_TRUE for mWantUpdates * + * seqno Current seqno value on the content side * anchor Offset where the selection started * focus Offset where the caret is */ - NotifyIMESelection(PRUint32 anchor, PRUint32 focus); + NotifyIMESelection(PRUint32 seqno, PRUint32 anchor, PRUint32 focus); /** * Notifies chrome to refresh its text cache diff --git a/dom/ipc/TabParent.cpp b/dom/ipc/TabParent.cpp index 2bd913bd689..33a9cad9b76 100644 --- a/dom/ipc/TabParent.cpp +++ b/dom/ipc/TabParent.cpp @@ -317,12 +317,14 @@ TabParent::RecvAsyncMessage(const nsString& aMessage, bool TabParent::RecvNotifyIMEFocus(const PRBool& aFocus, - nsIMEUpdatePreference* aPreference) + nsIMEUpdatePreference* aPreference, + PRUint32* aSeqno) { nsCOMPtr widget = GetWidget(); if (!widget) return true; + *aSeqno = mIMESeqno; mIMETabParent = aFocus ? this : nsnull; mIMESelectionAnchor = 0; mIMESelectionFocus = 0; @@ -355,16 +357,19 @@ TabParent::RecvNotifyIMETextChange(const PRUint32& aStart, } bool -TabParent::RecvNotifyIMESelection(const PRUint32& aAnchor, +TabParent::RecvNotifyIMESelection(const PRUint32& aSeqno, + const PRUint32& aAnchor, const PRUint32& aFocus) { nsCOMPtr widget = GetWidget(); if (!widget) return true; - mIMESelectionAnchor = aAnchor; - mIMESelectionFocus = aFocus; - widget->OnIMESelectionChange(); + if (aSeqno == mIMESeqno) { + mIMESelectionAnchor = aAnchor; + mIMESelectionFocus = aFocus; + widget->OnIMESelectionChange(); + } return true; } @@ -444,12 +449,13 @@ TabParent::HandleQueryContentEvent(nsQueryContentEvent& aEvent) } bool -TabParent::SendCompositionEvent(const nsCompositionEvent& event) +TabParent::SendCompositionEvent(nsCompositionEvent& event) { mIMEComposing = event.message == NS_COMPOSITION_START; mIMECompositionStart = PR_MIN(mIMESelectionAnchor, mIMESelectionFocus); if (mIMECompositionEnding) return true; + event.seqno = ++mIMESeqno; return PBrowserParent::SendCompositionEvent(event); } @@ -461,7 +467,7 @@ TabParent::SendCompositionEvent(const nsCompositionEvent& event) * here and pass the text as the EndIMEComposition return value */ bool -TabParent::SendTextEvent(const nsTextEvent& event) +TabParent::SendTextEvent(nsTextEvent& event) { if (mIMECompositionEnding) { mIMECompositionText = event.theText; @@ -476,14 +482,16 @@ TabParent::SendTextEvent(const nsTextEvent& event) mIMESelectionAnchor = mIMESelectionFocus = mIMECompositionStart + event.theText.Length(); + event.seqno = ++mIMESeqno; return PBrowserParent::SendTextEvent(event); } bool -TabParent::SendSelectionEvent(const nsSelectionEvent& event) +TabParent::SendSelectionEvent(nsSelectionEvent& event) { mIMESelectionAnchor = event.mOffset + (event.mReversed ? event.mLength : 0); mIMESelectionFocus = event.mOffset + (!event.mReversed ? event.mLength : 0); + event.seqno = ++mIMESeqno; return PBrowserParent::SendSelectionEvent(event); } diff --git a/dom/ipc/TabParent.h b/dom/ipc/TabParent.h index e140f8010dd..cb620da08da 100644 --- a/dom/ipc/TabParent.h +++ b/dom/ipc/TabParent.h @@ -92,11 +92,13 @@ public: virtual bool RecvAsyncMessage(const nsString& aMessage, const nsString& aJSON); virtual bool RecvNotifyIMEFocus(const PRBool& aFocus, - nsIMEUpdatePreference* aPreference); + nsIMEUpdatePreference* aPreference, + PRUint32* aSeqno); virtual bool RecvNotifyIMETextChange(const PRUint32& aStart, const PRUint32& aEnd, const PRUint32& aNewEnd); - virtual bool RecvNotifyIMESelection(const PRUint32& aAnchor, + virtual bool RecvNotifyIMESelection(const PRUint32& aSeqno, + const PRUint32& aAnchor, const PRUint32& aFocus); virtual bool RecvNotifyIMETextHint(const nsString& aText); virtual bool RecvEndIMEComposition(const PRBool& aCancel, @@ -179,9 +181,9 @@ public: static TabParent *GetIMETabParent() { return mIMETabParent; } bool HandleQueryContentEvent(nsQueryContentEvent& aEvent); - bool SendCompositionEvent(const nsCompositionEvent& event); - bool SendTextEvent(const nsTextEvent& event); - bool SendSelectionEvent(const nsSelectionEvent& event); + bool SendCompositionEvent(nsCompositionEvent& event); + bool SendTextEvent(nsTextEvent& event); + bool SendSelectionEvent(nsSelectionEvent& event); protected: bool ReceiveMessage(const nsString& aMessage, PRBool aSync, @@ -232,6 +234,7 @@ protected: // Compositions in almost all cases are small enough for nsAutoString nsAutoString mIMECompositionText; PRUint32 mIMECompositionStart; + PRUint32 mIMESeqno; private: already_AddRefed GetFrameLoader() const; diff --git a/widget/public/nsGUIEvent.h b/widget/public/nsGUIEvent.h index 13f952531d6..1334cab5e9b 100644 --- a/widget/public/nsGUIEvent.h +++ b/widget/public/nsGUIEvent.h @@ -1063,6 +1063,9 @@ private: nsTextEvent() { } + +public: + PRUint32 seqno; #endif // MOZ_IPC public: @@ -1091,6 +1094,9 @@ private: nsCompositionEvent() { } + +public: + PRUint32 seqno; #endif // MOZ_IPC public: @@ -1309,6 +1315,9 @@ private: nsSelectionEvent() { } + +public: + PRUint32 seqno; #endif // MOZ_IPC public: diff --git a/widget/public/nsGUIEventIPC.h b/widget/public/nsGUIEventIPC.h index 7b70f9a481f..0e31147000f 100644 --- a/widget/public/nsGUIEventIPC.h +++ b/widget/public/nsGUIEventIPC.h @@ -165,6 +165,7 @@ struct ParamTraits static void Write(Message* aMsg, const paramType& aParam) { WriteParam(aMsg, static_cast(aParam)); + WriteParam(aMsg, aParam.seqno); WriteParam(aMsg, aParam.theText); WriteParam(aMsg, aParam.isChar); WriteParam(aMsg, aParam.rangeCount); @@ -175,6 +176,7 @@ struct ParamTraits static bool Read(const Message* aMsg, void** aIter, paramType* aResult) { if (!ReadParam(aMsg, aIter, static_cast(aResult)) || + !ReadParam(aMsg, aIter, &aResult->seqno) || !ReadParam(aMsg, aIter, &aResult->theText) || !ReadParam(aMsg, aIter, &aResult->isChar) || !ReadParam(aMsg, aIter, &aResult->rangeCount)) @@ -212,11 +214,13 @@ struct ParamTraits static void Write(Message* aMsg, const paramType& aParam) { WriteParam(aMsg, static_cast(aParam)); + WriteParam(aMsg, aParam.seqno); } static bool Read(const Message* aMsg, void** aIter, paramType* aResult) { - return ReadParam(aMsg, aIter, static_cast(aResult)); + return ReadParam(aMsg, aIter, static_cast(aResult)) && + ReadParam(aMsg, aIter, &aResult->seqno); } }; @@ -261,6 +265,7 @@ struct ParamTraits static void Write(Message* aMsg, const paramType& aParam) { WriteParam(aMsg, static_cast(aParam)); + WriteParam(aMsg, aParam.seqno); WriteParam(aMsg, aParam.mOffset); WriteParam(aMsg, aParam.mLength); WriteParam(aMsg, aParam.mReversed); @@ -271,6 +276,7 @@ struct ParamTraits static bool Read(const Message* aMsg, void** aIter, paramType* aResult) { return ReadParam(aMsg, aIter, static_cast(aResult)) && + ReadParam(aMsg, aIter, &aResult->seqno) && ReadParam(aMsg, aIter, &aResult->mOffset) && ReadParam(aMsg, aIter, &aResult->mLength) && ReadParam(aMsg, aIter, &aResult->mReversed) && diff --git a/widget/src/xpwidgets/PuppetWidget.cpp b/widget/src/xpwidgets/PuppetWidget.cpp index f4026f1ca28..ac39f3794a7 100644 --- a/widget/src/xpwidgets/PuppetWidget.cpp +++ b/widget/src/xpwidgets/PuppetWidget.cpp @@ -111,7 +111,8 @@ PuppetWidget::Create(nsIWidget *aParent, gfxASurface::ContentFromFormat(gfxASurface::ImageFormatARGB32)); mIMEComposing = PR_FALSE; - mIMESuppressNotifySel = PR_FALSE; + mIMELastReceivedSeqno = 0; + mIMELastBlurSeqno = 0; PuppetWidget* parent = static_cast(aParent); if (parent) { @@ -277,15 +278,28 @@ PuppetWidget::DispatchEvent(nsGUIEvent* event, nsEventStatus& aStatus) if (mEventCallback) { if (event->message == NS_COMPOSITION_START) { mIMEComposing = PR_TRUE; - } else if (event->message == NS_SELECTION_SET) { - mIMESuppressNotifySel = PR_TRUE; + } + switch (event->eventStructType) { + case NS_COMPOSITION_EVENT: + mIMELastReceivedSeqno = static_cast(event)->seqno; + if (mIMELastReceivedSeqno < mIMELastBlurSeqno) + return NS_OK; + break; + case NS_TEXT_EVENT: + mIMELastReceivedSeqno = static_cast(event)->seqno; + if (mIMELastReceivedSeqno < mIMELastBlurSeqno) + return NS_OK; + break; + case NS_SELECTION_EVENT: + mIMELastReceivedSeqno = static_cast(event)->seqno; + if (mIMELastReceivedSeqno < mIMELastBlurSeqno) + return NS_OK; + break; } aStatus = (*mEventCallback)(event); if (event->message == NS_COMPOSITION_END) { mIMEComposing = PR_FALSE; - } else if (event->message == NS_SELECTION_SET) { - mIMESuppressNotifySel = PR_FALSE; } } else if (mChild) { event->widget = mChild; @@ -401,9 +415,10 @@ PuppetWidget::OnIMEFocusChange(PRBool aFocus) ResetInputState(); } + PRUint32 chromeSeqno; mIMEPreference.mWantUpdates = PR_FALSE; mIMEPreference.mWantHints = PR_FALSE; - if (!mTabChild->SendNotifyIMEFocus(aFocus, &mIMEPreference)) + if (!mTabChild->SendNotifyIMEFocus(aFocus, &mIMEPreference, &chromeSeqno)) return NS_ERROR_FAILURE; if (aFocus) { @@ -411,6 +426,8 @@ PuppetWidget::OnIMEFocusChange(PRBool aFocus) // call OnIMEFocusChange on blur but no other updates return NS_SUCCESS_IME_NO_UPDATES; OnIMESelectionChange(); // Update selection + } else { + mIMELastBlurSeqno = chromeSeqno; } return NS_OK; } @@ -444,13 +461,6 @@ PuppetWidget::OnIMESelectionChange(void) if (!mTabChild) return NS_ERROR_FAILURE; - // When we send selection notifications during a composition or during a - // set selection event, there is a race condition where the notification - // arrives at chrome too late, which leads to chrome thinking the - // selection was elsewhere. Suppress notifications here to avoid that. - if (mIMEComposing || mIMESuppressNotifySel) - return NS_OK; - if (mIMEPreference.mWantUpdates) { nsEventStatus status; nsQueryContentEvent queryEvent(PR_TRUE, NS_QUERY_SELECTED_TEXT, this); @@ -458,7 +468,8 @@ PuppetWidget::OnIMESelectionChange(void) DispatchEvent(&queryEvent, status); if (queryEvent.mSucceeded) { - mTabChild->SendNotifyIMESelection(queryEvent.GetSelectionStart(), + mTabChild->SendNotifyIMESelection(mIMELastReceivedSeqno, + queryEvent.GetSelectionStart(), queryEvent.GetSelectionEnd()); } } diff --git a/widget/src/xpwidgets/PuppetWidget.h b/widget/src/xpwidgets/PuppetWidget.h index 4eedd062da9..5f335636cfd 100644 --- a/widget/src/xpwidgets/PuppetWidget.h +++ b/widget/src/xpwidgets/PuppetWidget.h @@ -216,7 +216,13 @@ private: // IME nsIMEUpdatePreference mIMEPreference; PRPackedBool mIMEComposing; - PRPackedBool mIMESuppressNotifySel; + // Latest seqno received through events + PRUint32 mIMELastReceivedSeqno; + // Chrome's seqno value when last blur occurred + // arriving events with seqno up to this should be discarded + // Note that if seqno overflows (~50 days at 1 ms increment rate), + // events will be discarded until new focus/blur occurs + PRUint32 mIMELastBlurSeqno; }; } // namespace widget