author | Masayuki Nakano <masayuki@d-toybox.com> |
Mon, 20 Nov 2017 22:59:04 +0900 | |
changeset 393065 | e8d7881af680c693612c2ed42d3486c5ad4c8e42 |
parent 393064 | 21522127515146fb7742f4e52c5d4c860bc71562 |
child 393066 | 3e4e219b8aebf8e439d8ac40738321d7c7c81c6f |
push id | 55827 |
push user | masayuki@d-toybox.com |
push date | Wed, 22 Nov 2017 12:53:08 +0000 |
treeherder | autoland@7a3e5d976499 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | m_kato |
bugs | 1405832 |
milestone | 59.0a1 |
first release with | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
last release without | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
--- a/dom/events/TextComposition.cpp +++ b/dom/events/TextComposition.cpp @@ -60,16 +60,17 @@ TextComposition::TextComposition(nsPresC , mCompositionStartOffset(0) , mTargetClauseOffsetInComposition(0) , mIsSynthesizedForTests(aCompositionEvent->mFlags.mIsSynthesizedForTests) , mIsComposing(false) , mIsEditorHandlingEvent(false) , mIsRequestingCommit(false) , mIsRequestingCancel(false) , mRequestedToCommitOrCancel(false) + , mHasReceivedCommitEvent(false) , mWasNativeCompositionEndEventDiscarded(false) , mAllowControlCharacters( Preferences::GetBool("dom.compositionevent.allow_control_characters", false)) , mWasCompositionStringEmpty(true) { MOZ_ASSERT(aCompositionEvent->mNativeIMEContext.IsValid()); } @@ -241,16 +242,20 @@ void TextComposition::DispatchCompositionEvent( WidgetCompositionEvent* aCompositionEvent, nsEventStatus* aStatus, EventDispatchingCallback* aCallBack, bool aIsSynthesized) { mWasCompositionStringEmpty = mString.IsEmpty(); + if (aCompositionEvent->IsFollowedByCompositionEnd()) { + mHasReceivedCommitEvent = true; + } + // If this instance has requested to commit or cancel composition but // is not synthesizing commit event, that means that the IME commits or // cancels the composition asynchronously. Typically, iBus behaves so. // Then, synthesized events which were dispatched immediately after // the request has already committed our editor's composition string and // told it to web apps. Therefore, we should ignore the delayed events. if (mRequestedToCommitOrCancel && !aIsSynthesized) { *aStatus = nsEventStatus_eConsumeNoDefault; @@ -558,20 +563,22 @@ TextComposition::DispatchCompositionEven new CompositionEventDispatcher(this, mNode, aEventMessage, aData, aIsSynthesizingCommit)); } nsresult TextComposition::RequestToCommit(nsIWidget* aWidget, bool aDiscard) { // If this composition is already requested to be committed or canceled, - // we don't need to request it again because even if the first request - // failed, new request won't success, probably. And we shouldn't synthesize - // events for committing or canceling composition twice or more times. - if (mRequestedToCommitOrCancel) { + // or has already finished in IME, we don't need to request it again because + // request from this instance shouldn't cause committing nor canceling current + // composition in IME, and even if the first request failed, new request + // won't success, probably. And we shouldn't synthesize events for + // committing or canceling composition twice or more times. + if (!CanRequsetIMEToCommitOrCancelComposition()) { return NS_OK; } RefPtr<TextComposition> kungFuDeathGrip(this); const nsAutoString lastData(mLastData); { AutoRestore<bool> saveRequestingCancel(mIsRequestingCancel);
--- a/dom/events/TextComposition.h +++ b/dom/events/TextComposition.h @@ -96,16 +96,25 @@ public: /** * Request to commit (or cancel) the composition to IME. This method should * be called only by IMEStateManager::NotifyIME(). */ nsresult RequestToCommit(nsIWidget* aWidget, bool aDiscard); /** + * IsRequestingCommitOrCancelComposition() returns true if the instance is + * requesting widget to commit or cancel composition. + */ + bool IsRequestingCommitOrCancelComposition() const + { + return mIsRequestingCancel || mIsRequestingCommit; + } + + /** * Send a notification to IME. It depends on the IME or platform spec what * will occur (or not occur). */ nsresult NotifyIME(widget::IMEMessage aMessage); /** * the offset of first composition string */ @@ -242,20 +251,24 @@ private: // requesting commit or canceling the composition. In other words, while // one of these values is true, we're handling the request. bool mIsRequestingCommit; bool mIsRequestingCancel; // mRequestedToCommitOrCancel is true *after* we requested IME to commit or // cancel the composition. In other words, we already requested of IME that // it commits or cancels current composition. - // NOTE: Before this is set true, both mIsRequestingCommit and - // mIsRequestingCancel are set false. + // NOTE: Before this is set to true, both mIsRequestingCommit and + // mIsRequestingCancel are set to false. bool mRequestedToCommitOrCancel; + // Before this dispatches commit event into the tree, this is set to true. + // So, this means if native IME already commits the composition. + bool mHasReceivedCommitEvent; + // mWasNativeCompositionEndEventDiscarded is true if this composition was // requested commit or cancel itself but native compositionend event is // discarded by PresShell due to not safe to dispatch events. bool mWasNativeCompositionEndEventDiscarded; // Allow control characters appear in composition string. // When this is false, control characters except // CHARACTER TABULATION (horizontal tab) are removed from @@ -274,23 +287,38 @@ private: , mCompositionStartOffset(0) , mTargetClauseOffsetInComposition(0) , mIsSynthesizedForTests(false) , mIsComposing(false) , mIsEditorHandlingEvent(false) , mIsRequestingCommit(false) , mIsRequestingCancel(false) , mRequestedToCommitOrCancel(false) + , mHasReceivedCommitEvent(false) , mWasNativeCompositionEndEventDiscarded(false) , mAllowControlCharacters(false) , mWasCompositionStringEmpty(true) {} TextComposition(const TextComposition& aOther); /** + * If we're requesting IME to commit or cancel composition, or we've already + * requested it, or we've already known this composition has been ended in + * IME, we don't need to request commit nor cancel composition anymore and + * shouldn't do so if we're in content process for not committing/canceling + * "current" composition in native IME. So, when this returns true, + * RequestIMEToCommit() does nothing. + */ + bool CanRequsetIMEToCommitOrCancelComposition() const + { + return !mIsRequestingCommit && !mIsRequestingCancel && + !mRequestedToCommitOrCancel && !mHasReceivedCommitEvent; + } + + /** * GetEditorBase() returns EditorBase pointer of mEditorBaseWeak. */ already_AddRefed<EditorBase> GetEditorBase() const; /** * HasEditor() returns true if mEditorBaseWeak holds EditorBase instance * which is alive. Otherwise, false. */
--- a/widget/PuppetWidget.cpp +++ b/widget/PuppetWidget.cpp @@ -647,16 +647,20 @@ PuppetWidget::RequestIMEToCommitComposit RefPtr<TextComposition> composition = IMEStateManager::GetTextCompositionFor(this); // This method shouldn't be called when there is no text composition instance. if (NS_WARN_IF(!composition)) { return NS_OK; } + MOZ_DIAGNOSTIC_ASSERT(composition->IsRequestingCommitOrCancelComposition(), + "Requesting commit or cancel composition should be requested via " + "TextComposition instance"); + bool isCommitted = false; nsAutoString committedString; if (NS_WARN_IF(!mTabChild->SendRequestIMEToCommitComposition( aCancel, &isCommitted, &committedString))) { return NS_ERROR_FAILURE; } // If the composition wasn't committed synchronously, we need to wait async