From fd30eaf3654811c5ad8834f2571baafb1e5c8744 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Mon, 7 Feb 2022 22:33:37 +0000 Subject: [PATCH] Bug 1746104 - part 5-1: Get rid of `WidgetQueryContentEvent::Reply::mHasSelection` r=m_kato It's intended to indicate whether the selection is collapsed or not, but it can be referred by other members, there is no reasonable user and the name makes developers confused. Differential Revision: https://phabricator.services.mozilla.com/D137422 --- dom/events/ContentEventHandler.cpp | 6 +- dom/events/IMEContentObserver.cpp | 1 - widget/ContentCache.cpp | 1 - widget/TextEvents.h | 6 +- widget/moz.build | 3 + ...window_composition_text_querycontent.xhtml | 79 ++++++++++++++++--- 6 files changed, 75 insertions(+), 21 deletions(-) diff --git a/dom/events/ContentEventHandler.cpp b/dom/events/ContentEventHandler.cpp index 0904bff65a93..5d7b85bd12b1 100644 --- a/dom/events/ContentEventHandler.cpp +++ b/dom/events/ContentEventHandler.cpp @@ -423,8 +423,6 @@ nsresult ContentEventHandler::Init(WidgetQueryContentEvent* aEvent) { aEvent->mReply->mContentsRoot = mRootContent.get(); - aEvent->mReply->mHasSelection = !mSelection->IsCollapsed(); - nsRect r; nsIFrame* frame = nsCaret::GetGeometry(mSelection, &r); if (!frame) { @@ -1321,7 +1319,7 @@ nsresult ContentEventHandler::OnQuerySelectedText( if (!mFirstSelectedRawRange.IsPositioned()) { MOZ_ASSERT(aEvent->mInput.mSelectionType != SelectionType::eNormal); MOZ_ASSERT(aEvent->mReply->mOffsetAndData.isNothing()); - MOZ_ASSERT(!aEvent->mReply->mHasSelection); + MOZ_ASSERT_IF(mSelection, !mSelection->RangeCount()); // This is special case that `mReply` is emplaced, but mOffsetAndData is // not emplaced but treated as succeeded because of no selection ranges // is a usual case. @@ -2546,7 +2544,7 @@ nsresult ContentEventHandler::OnQuerySelectionAsTransferable( MOZ_ASSERT(aEvent->mReply.isSome()); - if (!aEvent->mReply->mHasSelection) { + if (mSelection->IsCollapsed()) { MOZ_ASSERT(!aEvent->mReply->mTransferable); return NS_OK; } diff --git a/dom/events/IMEContentObserver.cpp b/dom/events/IMEContentObserver.cpp index 84739f934710..ab7c7219b898 100644 --- a/dom/events/IMEContentObserver.cpp +++ b/dom/events/IMEContentObserver.cpp @@ -607,7 +607,6 @@ nsresult IMEContentObserver::HandleQueryContentEvent( mSelectionData.String(), OffsetAndDataFor::SelectedString); aEvent->mReply->mContentsRoot = mRootContent; - aEvent->mReply->mHasSelection = !mSelectionData.IsCollapsed(); aEvent->mReply->mWritingMode = mSelectionData.GetWritingMode(); aEvent->mReply->mReversed = mSelectionData.mReversed; MOZ_LOG(sIMECOLog, LogLevel::Debug, diff --git a/widget/ContentCache.cpp b/widget/ContentCache.cpp index f078cd38c550..af269da14502 100644 --- a/widget/ContentCache.cpp +++ b/widget/ContentCache.cpp @@ -688,7 +688,6 @@ bool ContentCacheInParent::HandleQueryContentEvent( Substring(mText, mSelection->StartOffset(), mSelection->Length()), OffsetAndDataFor::SelectedString); aEvent.mReply->mReversed = mSelection->Reversed(); - aEvent.mReply->mHasSelection = true; aEvent.mReply->mWritingMode = mSelection->mWritingMode; MOZ_LOG(sContentCacheLog, LogLevel::Info, ("0x%p HandleQueryContentEvent(), " diff --git a/widget/TextEvents.h b/widget/TextEvents.h index d505cf28c2f8..62c342aa8d4f 100644 --- a/widget/TextEvents.h +++ b/widget/TextEvents.h @@ -1173,8 +1173,6 @@ class WidgetQueryContentEvent : public WidgetGUIEvent { CopyableTArray mRectArray; // true if selection is reversed (end < start) bool mReversed; - // true if the selection exists - bool mHasSelection; // true if DOM element under mouse belongs to widget bool mWidgetIsHit; @@ -1184,7 +1182,6 @@ class WidgetQueryContentEvent : public WidgetGUIEvent { mContentsRoot(nullptr), mFocusedWidget(nullptr), mReversed(false), - mHasSelection(false), mWidgetIsHit(false) {} // Don't allow to copy/move because of `mEventMessage`. @@ -1272,8 +1269,7 @@ class WidgetQueryContentEvent : public WidgetGUIEvent { << ToString(aReply.mTentativeCaretOffset).c_str() << ", "; } } - aStream << "mHasSelection=" << (aReply.mHasSelection ? "true" : "false"); - if (aReply.mHasSelection) { + if (aReply.mOffsetAndData.isSome() && aReply.mOffsetAndData->Length()) { if (aReply.mEventMessage == eQuerySelectedText) { aStream << ", mReversed=" << (aReply.mReversed ? "true" : "false"); } diff --git a/widget/moz.build b/widget/moz.build index c354bc4d7aec..45f6d292bbc2 100644 --- a/widget/moz.build +++ b/widget/moz.build @@ -28,6 +28,9 @@ with Files("*CompositorWidget*"): with Files("*ContentData*"): BUG_COMPONENT = ("Core", "DOM: UI Events & Focus Handling") +with Files("*Events.h"): + BUG_COMPONENT = ("Core", "DOM: UI Events & Focus Handling") + with Files("*FontRange*"): BUG_COMPONENT = ("Core", "Widget: Cocoa") diff --git a/widget/tests/window_composition_text_querycontent.xhtml b/widget/tests/window_composition_text_querycontent.xhtml index e418e11a5740..691faa8bf678 100644 --- a/widget/tests/window_composition_text_querycontent.xhtml +++ b/widget/tests/window_composition_text_querycontent.xhtml @@ -203,10 +203,33 @@ function checkSelection(aExpectedOffset, aExpectedText, aMessage, aID) ": synthesizeQuerySelectedText " + aID)) { return false; } - is(selectedText.offset, aExpectedOffset, - aMessage + ": selection offset is wrong " + aID); - is(selectedText.text, aExpectedText, - aMessage + ": selected text is wrong " + aID); + if (aExpectedOffset === null) { + todo_is( + selectedText.notFound, + true, + `${aMessage}: selection should not be found ${aID}` + ); + return selectedText.notFound; + } + + is( + selectedText.notFound, + false, + `${aMessage}: selection should be found ${aID}` + ); + if (selectedText.notFound) { + return false; + } + is( + selectedText.offset, + aExpectedOffset, + `${aMessage}: selection offset should be ${aExpectedOffset} ${aID}` + ); + is( + selectedText.text, + aExpectedText, + `${aMessage}: selected text should be "${aExpectedText}" ${aID}` + ); return selectedText.offset == aExpectedOffset && selectedText.text == aExpectedText; } @@ -4551,18 +4574,54 @@ function runQuerySelectionEventTest() // #1 contenteditable.innerHTML = "
a"; - selection.setBaseAndExtent(contenteditable.firstChild, 0, contenteditable.lastChild, 1); - checkSelection(0, kLF + "a", "runQuerySelectionEventTest #1, \"" + contenteditable.innerHTML + "\""); + selection.setBaseAndExtent( + contenteditable.firstChild, + 0, + contenteditable.lastChild, + 1 + ); + checkSelection( + 0, + `${kLF}a`, + `runQuerySelectionEventTest #1, "${contenteditable.innerHTML}"` + ); // #2 contenteditable.innerHTML = "

abc

"; - selection.setBaseAndExtent(contenteditable.firstChild, 0, contenteditable.lastChild.firstChild, 1); - checkSelection(kLFLen, kLF + "a", "runQuerySelectionEventTest #2, \"" + contenteditable.innerHTML + "\""); + selection.setBaseAndExtent( + contenteditable.firstChild, + 0, + contenteditable.lastChild.firstChild, + 1 + ); + checkSelection( + kLFLen, + `${kLF}a`, + `runQuerySelectionEventTest #2, "${contenteditable.innerHTML}"` + ); // #3 contenteditable.innerHTML = "

abc

def

"; - selection.setBaseAndExtent(contenteditable.firstChild, 0, contenteditable.lastChild.firstChild, 1); - checkSelection(kLFLen, "abc" + kLF + "d", "runQuerySelectionEventTest #3, \"" + contenteditable.innerHTML + "\""); + selection.setBaseAndExtent( + contenteditable.firstChild, + 0, + contenteditable.lastChild.firstChild, + 1 + ); + checkSelection( + kLFLen, + `abc${kLF}d`, + `runQuerySelectionEventTest #3, "${contenteditable.innerHTML}"` + ); + + // #4 + contenteditable.innerHTML = "

abc

"; + selection.removeAllRanges(); + checkSelection( + null, + null, + `runQuerySelectionEventTest #4, "${contenteditable.innerHTML}"` + ); } function runQueryIMESelectionTest()