From 91e18f14d875cca4b45618e58a5b9f6a84b564c6 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Thu, 7 Apr 2022 00:58:49 +0000 Subject: [PATCH] Bug 1760160 - Make `ContentCacheInChild` stop storing content if editable element has already been blurred r=m_kato It's designed for caching content information of focused editor. However, at sending focus notification to the main process, the editor may have already been blurred but `IMEContentObserver` may have not known it yet. In this edge case, only query selection succeeds since IMEContentObserver still has a cache, but query the others failed because of root content node check of `IMEContentObserver::HandleQueryContentEvent`. If a content process meets this case, it should not send focus notification and stop storing the content since IME shouldn't get focus nor query non-editable content. On the other hand, the reported testcase reproduces this with a fuzzing API called **in** the content process. Therefore, I have no idea how to reproduce it without the API. That's the reason why this patch does not contain new tests. Differential Revision: https://phabricator.services.mozilla.com/D141821 --- dom/events/ContentEventHandler.cpp | 2 ++ dom/events/IMEContentObserver.cpp | 6 ++++ widget/ContentCache.cpp | 48 ++++++++++++++++++++++++++++-- widget/PuppetWidget.cpp | 3 +- widget/TextEvents.h | 19 ++++++------ 5 files changed, 65 insertions(+), 13 deletions(-) diff --git a/dom/events/ContentEventHandler.cpp b/dom/events/ContentEventHandler.cpp index 27e5d655c18f..0bc5b66bf315 100644 --- a/dom/events/ContentEventHandler.cpp +++ b/dom/events/ContentEventHandler.cpp @@ -430,6 +430,8 @@ nsresult ContentEventHandler::Init(WidgetQueryContentEvent* aEvent) { aEvent->EmplaceReply(); aEvent->mReply->mContentsRoot = mRootContent.get(); + aEvent->mReply->mIsEditableContent = + mRootContent && mRootContent->IsEditable(); nsRect r; nsIFrame* frame = nsCaret::GetGeometry(mSelection, &r); diff --git a/dom/events/IMEContentObserver.cpp b/dom/events/IMEContentObserver.cpp index 485b8b561f6f..f21348f0c4a8 100644 --- a/dom/events/IMEContentObserver.cpp +++ b/dom/events/IMEContentObserver.cpp @@ -607,6 +607,12 @@ nsresult IMEContentObserver::HandleQueryContentEvent( } aEvent->mReply->mContentsRoot = mRootContent; aEvent->mReply->mWritingMode = mSelectionData.GetWritingMode(); + // The selection cache in IMEContentObserver must always have been in + // an editing host (or an editable annoymous
element). Therefore, + // we set mIsEditableContent to true here even though it's already been + // blurred or changed its editable state but the selection cache has not + // been invalidated yet. + aEvent->mReply->mIsEditableContent = true; MOZ_LOG(sIMECOLog, LogLevel::Debug, ("0x%p HandleQueryContentEvent(aEvent={ " "mMessage=%s, mReply=%s })", diff --git a/widget/ContentCache.cpp b/widget/ContentCache.cpp index dc4bc7e480f0..65ac8c0b7557 100644 --- a/widget/ContentCache.cpp +++ b/widget/ContentCache.cpp @@ -133,6 +133,20 @@ bool ContentCacheInChild::CacheSelection(nsIWidget* aWidget, sContentCacheLog, LogLevel::Error, ("0x%p CacheSelection(), FAILED, couldn't retrieve the selected text", this)); + } + // ContentCache should store only editable content. Therefore, if current + // selection root is not editable, we don't need to store the selection, i.e., + // let's treat it as there is no selection. However, if we already have + // previously editable text, let's store the selection even if it becomes + // uneditable because not doing so would create odd situation. E.g., IME may + // fail only querying selection after succeeded querying text. + else if (NS_WARN_IF(mText.isNothing() && + !querySelectedTextEvent.mReply->mIsEditableContent)) { + MOZ_LOG(sContentCacheLog, LogLevel::Error, + ("0x%p CacheSelection(), FAILED, editable content had already been " + "blurred", + this)); + return false; } else { mSelection.emplace(querySelectedTextEvent); } @@ -193,6 +207,16 @@ bool ContentCacheInChild::CacheEditorRect( this)); return false; } + // ContentCache should store only editable content. Therefore, if current + // selection root is not editable, we don't need to store the editor rect, + // i.e., let's treat it as there is no focused editor. + if (NS_WARN_IF(!queryEditorRectEvent.mReply->mIsEditableContent)) { + MOZ_LOG(sContentCacheLog, LogLevel::Error, + ("0x%p CacheText(), FAILED, editable content had already been " + "blurred", + this)); + return false; + } mEditorRect = queryEditorRectEvent.mReply->mRect; MOZ_LOG(sContentCacheLog, LogLevel::Info, ("0x%p CacheEditorRect(), Succeeded, mEditorRect=%s", this, @@ -215,6 +239,16 @@ bool ContentCacheInChild::CacheText(nsIWidget* aWidget, MOZ_LOG(sContentCacheLog, LogLevel::Error, ("0x%p CacheText(), FAILED, couldn't retrieve whole text", this)); mText.reset(); + } + // ContentCache should store only editable content. Therefore, if current + // selection root is not editable, we don't need to store the text, i.e., + // let's treat it as there is no editable text. + else if (NS_WARN_IF(!queryTextContentEvent.mReply->mIsEditableContent)) { + MOZ_LOG(sContentCacheLog, LogLevel::Error, + ("0x%p CacheText(), FAILED, editable content had already been " + "blurred", + this)); + mText.reset(); } else { mText = Some(nsString(queryTextContentEvent.mReply->DataRef())); MOZ_LOG(sContentCacheLog, LogLevel::Info, @@ -241,8 +275,18 @@ bool ContentCacheInChild::CacheText(nsIWidget* aWidget, mLastCommit.reset(); } - const bool selectionCached = CacheSelection(aWidget, aNotification); - return selectionCached || queryTextContentEvent.Succeeded(); + // If we fail to get editable text content, it must mean that there is no + // focused element anymore or focused element is not editable. In this case, + // we should not get selection of non-editable content + if (MOZ_UNLIKELY(queryTextContentEvent.Failed() || + !queryTextContentEvent.mReply->mIsEditableContent)) { + mSelection.reset(); + mCaret.reset(); + mTextRectArray.reset(); + return false; + } + + return CacheSelection(aWidget, aNotification); } bool ContentCacheInChild::QueryCharRect(nsIWidget* aWidget, uint32_t aOffset, diff --git a/widget/PuppetWidget.cpp b/widget/PuppetWidget.cpp index 021f17c7839d..b157200ed37a 100644 --- a/widget/PuppetWidget.cpp +++ b/widget/PuppetWidget.cpp @@ -755,7 +755,8 @@ nsresult PuppetWidget::NotifyIMEOfFocusChange( bool gotFocus = aIMENotification.mMessage == NOTIFY_IME_OF_FOCUS; if (gotFocus) { // When IME gets focus, we should initialize all information of the - // content. + // content, however, it may fail to get it because the editor may have + // already been blurred. if (NS_WARN_IF(!mContentCache.CacheAll(this, &aIMENotification))) { return NS_ERROR_FAILURE; } diff --git a/widget/TextEvents.h b/widget/TextEvents.h index 6dbdd3686284..e079c51dc6bd 100644 --- a/widget/TextEvents.h +++ b/widget/TextEvents.h @@ -1146,7 +1146,7 @@ class WidgetQueryContentEvent : public WidgetGUIEvent { struct Reply final { EventMessage const mEventMessage; - void* mContentsRoot; + void* mContentsRoot = nullptr; Maybe> mOffsetAndData; // mTentativeCaretOffset is used by only eQueryCharacterAtPoint. // This is the offset where caret would be if user clicked at the mRefPoint. @@ -1158,7 +1158,7 @@ class WidgetQueryContentEvent : public WidgetGUIEvent { // to the owner window, not the . mozilla::LayoutDeviceIntRect mRect; // The return widget has the caret. This is set at all query events. - nsIWidget* mFocusedWidget; + nsIWidget* mFocusedWidget = nullptr; // mozilla::WritingMode value at the end (focus) of the selection mozilla::WritingMode mWritingMode; // Used by eQuerySelectionAsTransferable @@ -1168,17 +1168,14 @@ class WidgetQueryContentEvent : public WidgetGUIEvent { // Used by eQueryTextRectArray CopyableTArray mRectArray; // true if selection is reversed (end < start) - bool mReversed; + bool mReversed = false; // true if DOM element under mouse belongs to widget - bool mWidgetIsHit; + bool mWidgetIsHit = false; + // true if mContentRoot is focused editable content + bool mIsEditableContent = false; Reply() = delete; - explicit Reply(EventMessage aEventMessage) - : mEventMessage(aEventMessage), - mContentsRoot(nullptr), - mFocusedWidget(nullptr), - mReversed(false), - mWidgetIsHit(false) {} + explicit Reply(EventMessage aEventMessage) : mEventMessage(aEventMessage) {} // Don't allow to copy/move because of `mEventMessage`. Reply(const Reply& aOther) = delete; @@ -1279,6 +1276,8 @@ class WidgetQueryContentEvent : public WidgetGUIEvent { aStream << ", mWritingMode=" << ToString(aReply.mWritingMode).c_str(); } aStream << ", mContentsRoot=0x" << aReply.mContentsRoot + << ", mIsEditableContent=" + << (aReply.mIsEditableContent ? "true" : "false") << ", mFocusedWidget=0x" << aReply.mFocusedWidget; if (aReply.mEventMessage == eQueryTextContent) { aStream << ", mFontRanges={ Length()=" << aReply.mFontRanges.Length()