From d90c65dc4d83c037ab2a58614614f2a447a366a8 Mon Sep 17 00:00:00 2001 From: Marco Castelluccio Date: Thu, 27 Feb 2020 18:22:38 +0000 Subject: [PATCH] Bug 1616257 - part 1: Redesign `WSRunScanner::GetWSBoundingParent()` r=m_kato Its result has 4 meanings: 1. an editable block element for container of `mScanStartPoint`. 2. a topmost inline editable content for container of `mScanStartPoint` if there is no editable block parent. 3. container of `mScanStartPoint` if it's a block (either editable or non-editable). 4. container of `mScanStartPoint` if its parent is not editable and a inline content. #1, #2 and editable case of #3 make sense because the results are topmost editable content in current context. On the other hand, non-editable case of #3 and #4 are caused by unexpected wrong fallback code. So, let's make it always returns the content in the former meaning and if the caller needs the latter one, they should use the container by themselves. Therefore, for making what's the start of the search, this patch also makes new method take start content instead of hiding `mScanStartPoint` from the callers. Differential Revision: https://phabricator.services.mozilla.com/D63610 UltraBlame original commit: f8505320d2165dd86f8fbb212ffe87397a814aaf --- editor/libeditor/WSRunObject.cpp | 55 ++++++++++++++----- editor/libeditor/WSRunObject.h | 4 +- editor/libeditor/crashtests/crashtests.list | 6 +- editor/libeditor/tests/test_bug430392.html | 5 ++ .../meta/editing/run/inserthtml.html.ini | 2 + 5 files changed, 53 insertions(+), 19 deletions(-) diff --git a/editor/libeditor/WSRunObject.cpp b/editor/libeditor/WSRunObject.cpp index 8b0ba37c4fe2..0fb5bd3ae1c0 100644 --- a/editor/libeditor/WSRunObject.cpp +++ b/editor/libeditor/WSRunObject.cpp @@ -654,22 +654,26 @@ nsresult WSRunObject::AdjustWhitespace() { -nsINode* WSRunScanner::GetWSBoundingParent() const { - if (NS_WARN_IF(!mScanStartPoint.IsSet())) { +nsIContent* WSRunScanner::GetEditableBlockParentOrTopmotEditableInlineContent( + nsIContent* aContent) const { + if (NS_WARN_IF(!aContent)) { return nullptr; } + NS_ASSERTION(mHTMLEditor->IsEditable(aContent), + "Given content is not editable"); - OwningNonNull wsBoundingParent = *mScanStartPoint.GetContainer(); - while (!IsBlockNode(wsBoundingParent)) { - nsCOMPtr parent = wsBoundingParent->GetParentNode(); - if (!parent || !mHTMLEditor->IsEditable(parent)) { + nsIContent* editableBlockParentOrTopmotEditableInlineContent = nullptr; + for (nsIContent* content = aContent; + content && mHTMLEditor->IsEditable(content); + content = content->GetParent()) { + editableBlockParentOrTopmotEditableInlineContent = content; + if (IsBlockNode(editableBlockParentOrTopmotEditableInlineContent)) { break; } - wsBoundingParent = parent; } - return wsBoundingParent; + return editableBlockParentOrTopmotEditableInlineContent; } nsresult WSRunScanner::GetWSNodes() { @@ -677,7 +681,20 @@ nsresult WSRunScanner::GetWSNodes() { EditorDOMPoint start(mScanStartPoint), end(mScanStartPoint); - nsCOMPtr wsBoundingParent = GetWSBoundingParent(); + nsIContent* scanStartContent = mScanStartPoint.GetContainerAsContent(); + if (NS_WARN_IF(!scanStartContent)) { + + + + + return NS_ERROR_FAILURE; + } + nsIContent* editableBlockParentOrTopmotEditableInlineContent = + GetEditableBlockParentOrTopmotEditableInlineContent(scanStartContent); + if (NS_WARN_IF(!editableBlockParentOrTopmotEditableInlineContent)) { + + editableBlockParentOrTopmotEditableInlineContent = scanStartContent; + } if (Text* textNode = mScanStartPoint.GetContainerAsText()) { @@ -715,7 +732,8 @@ nsresult WSRunScanner::GetWSNodes() { while (!mStartNode) { - nsCOMPtr priorNode = GetPreviousWSNode(start, wsBoundingParent); + nsCOMPtr priorNode = GetPreviousWSNode( + start, editableBlockParentOrTopmotEditableInlineContent); if (priorNode) { if (IsBlockNode(priorNode)) { mStartNode = start.GetContainer(); @@ -777,10 +795,13 @@ nsresult WSRunScanner::GetWSNodes() { } } else { + mStartNode = start.GetContainer(); mStartOffset = start.Offset(); mStartReason = WSType::thisBlock; - mStartReasonNode = wsBoundingParent; + + + mStartReasonNode = editableBlockParentOrTopmotEditableInlineContent; } } @@ -820,7 +841,8 @@ nsresult WSRunScanner::GetWSNodes() { while (!mEndNode) { - nsCOMPtr nextNode = GetNextWSNode(end, wsBoundingParent); + nsCOMPtr nextNode = + GetNextWSNode(end, editableBlockParentOrTopmotEditableInlineContent); if (nextNode) { if (IsBlockNode(nextNode)) { @@ -884,10 +906,13 @@ nsresult WSRunScanner::GetWSNodes() { } } else { + mEndNode = end.GetContainer(); mEndOffset = end.Offset(); mEndReason = WSType::thisBlock; - mEndReasonNode = wsBoundingParent; + + + mEndReasonNode = editableBlockParentOrTopmotEditableInlineContent; } } @@ -1839,7 +1864,9 @@ nsresult WSRunObject::CheckTrailingNBSPOfRun(WSFragment* aRun) { rightCheck = true; } if ((aRun->mRightType & WSType::block) && - IsBlockNode(GetWSBoundingParent())) { + (IsBlockNode(GetEditableBlockParentOrTopmotEditableInlineContent( + mScanStartPoint.GetContainerAsContent())) || + IsBlockNode(mScanStartPoint.GetContainerAsContent()))) { RefPtr selection = htmlEditor->GetSelection(); if (NS_WARN_IF(!selection)) { return NS_ERROR_FAILURE; diff --git a/editor/libeditor/WSRunObject.h b/editor/libeditor/WSRunObject.h index d41353220869..8a7e21e3c182 100644 --- a/editor/libeditor/WSRunObject.h +++ b/editor/libeditor/WSRunObject.h @@ -295,8 +295,8 @@ class MOZ_STACK_CLASS WSRunScanner { - - nsINode* GetWSBoundingParent() const; + nsIContent* GetEditableBlockParentOrTopmotEditableInlineContent( + nsIContent* aContent) const; static bool IsBlockNode(nsINode* aNode); diff --git a/editor/libeditor/crashtests/crashtests.list b/editor/libeditor/crashtests/crashtests.list index ca010200f897..5fdd77d50176 100644 --- a/editor/libeditor/crashtests/crashtests.list +++ b/editor/libeditor/crashtests/crashtests.list @@ -80,7 +80,7 @@ load 1348851.html load 1350772.html load 1364133.html load 1366176.html -load 1375131.html +asserts(3) load 1375131.html # assertion in WSRunScanner::GetEditableBlockParentOrTopmotEditableInlineContent() load 1381541.html load 1383747.html load 1383755.html @@ -94,7 +94,7 @@ load 1402526.html pref(dom.document.exec_command.nested_calls_allowed,true) asserts(1) load 1402904.html # assertion is that mutation event listener caused by execCommand calls another execCommand pref(dom.document.exec_command.nested_calls_allowed,true) asserts(1) load 1405747.html # assertion is that mutation event listener caused by execCommand calls another execCommand load 1405897.html -load 1408170.html +asserts(3) load 1408170.html # assertion in WSRunScanner::GetEditableBlockParentOrTopmotEditableInlineContent() asserts(0-1) load 1414581.html load 1415231.html load 1423767.html @@ -111,7 +111,7 @@ pref(layout.accessiblecaret.enabled,true) load 1470926.html pref(dom.document.exec_command.nested_calls_allowed,true) asserts(1) load 1474978.html # assertion is that mutation event listener caused by execCommand calls another execCommand load 1525481.html load 1533913.html -load 1534394.html +asserts(4) load 1534394.html # assertion in WSRunScanner::GetEditableBlockParentOrTopmotEditableInlineContent() load 1547897.html load 1547898.html load 1556799.html diff --git a/editor/libeditor/tests/test_bug430392.html b/editor/libeditor/tests/test_bug430392.html index ddac5f6fe62f..59ea105f4d67 100644 --- a/editor/libeditor/tests/test_bug430392.html +++ b/editor/libeditor/tests/test_bug430392.html @@ -110,6 +110,11 @@ SimpleTest.finish(); } +// In WSRunScanner::GetEditableBlockParentOrTopmotEditableInlineContent(). +// Before it, HTMLEditor must be able to stop handling edit action at +// non-editable content. +SimpleTest.expectAssertions(18); + SimpleTest.waitForExplicitFinish(); SimpleTest.waitForFocus(test); diff --git a/testing/web-platform/meta/editing/run/inserthtml.html.ini b/testing/web-platform/meta/editing/run/inserthtml.html.ini index 06bfca8cd941..e8fdf4bbe1d6 100644 --- a/testing/web-platform/meta/editing/run/inserthtml.html.ini +++ b/testing/web-platform/meta/editing/run/inserthtml.html.ini @@ -1,4 +1,6 @@ [inserthtml.html] + min-asserts: 14 + max-asserts: 14 prefs: [editor.use_div_for_default_newlines:true] [[["stylewithcss","true"\],["inserthtml","abcd"\]\] "[foobar\]baz" compare innerHTML] expected: FAIL