Bug 1627175 - part 44: Add `HTMLEditUtils::IsInvisibleBRElement()` for avoiding a mistake r=m_kato

When I review patches, sometimes I saw wrong usage of `!IsVisibleBRElement()`.
It means that it's an invisible <br> element **or** non-<br> element node,
but developers may think it checks whether it's an invisible `<br>` element
or not without checking the specifying content is a <br> element.

For avoiding this, there should be `IsInvisibleBRElement()` too.

Depends on D114932

Differential Revision: https://phabricator.services.mozilla.com/D114933
This commit is contained in:
Masayuki Nakano 2021-05-13 06:46:24 +00:00
Родитель 5fe255d4f6
Коммит b9607b64d3
4 изменённых файлов: 21 добавлений и 22 удалений

Просмотреть файл

@ -2354,8 +2354,7 @@ EditorDOMPoint HTMLEditor::GetGoodCaretPointFor(
// If we are going backward, put caret to next node unless aContent is an
// invisible `<br>` element.
// XXX Shouldn't we put caret to first leaf of the next node?
if (!aContent.IsHTMLElement(nsGkAtoms::br) ||
HTMLEditUtils::IsVisibleBRElement(aContent)) {
if (!HTMLEditUtils::IsInvisibleBRElement(aContent)) {
EditorDOMPoint ret(EditorDOMPoint::After(aContent));
NS_WARNING_ASSERTION(ret.IsSet(), "Failed to set after aContent");
return ret;
@ -8203,8 +8202,8 @@ nsresult HTMLEditor::AdjustCaretPositionAndEnsurePaddingBRElement(
previousEditableContent->IsHTMLElement(nsGkAtoms::br)) {
// If it's an invisible `<br>` element, we need to insert a padding
// `<br>` element for making empty line have one-line height.
if (!HTMLEditUtils::IsVisibleBRElement(*previousEditableContent,
editingHost)) {
if (HTMLEditUtils::IsInvisibleBRElement(*previousEditableContent,
editingHost)) {
CreateElementResult createPaddingBRResult =
InsertPaddingBRElementForEmptyLastLineWithTransaction(point);
if (createPaddingBRResult.Failed()) {

Просмотреть файл

@ -263,9 +263,9 @@ class HTMLEditUtils final {
dom::Text& aText);
/**
* IsVisibleBRElement() returns true if aContent is a visible HTML <br>
* element, i.e., not a padding <br> element for making last line in a
* block element visible.
* IsVisibleBRElement() and IsInvisibleBRElement() return true if aContent is
* a visible HTML <br> element, i.e., not a padding <br> element for making
* last line in a block element visible, or an invisible <br> element.
*
* If aEditingHost is omitted, this computes parent editable block for you.
* But if you call this a lot, please specify proper editing host (or parent
@ -273,6 +273,11 @@ class HTMLEditUtils final {
*/
static bool IsVisibleBRElement(const nsIContent& aContent,
const Element* aEditingHost = nullptr);
static bool IsInvisibleBRElement(const nsIContent& aContent,
const Element* aEditingHost = nullptr) {
return aContent.IsHTMLElement(nsGkAtoms::br) &&
!HTMLEditUtils::IsVisibleBRElement(aContent, aEditingHost);
}
/**
* IsEmptyNode() returns false if aNode has some visible content nodes,

Просмотреть файл

@ -340,10 +340,8 @@ HTMLEditor::HTMLWithContextInserter::GetInvisibleBRElementAtPoint(
WSRunScanner wsRunScannerAtInsertionPoint(mHTMLEditor.GetActiveEditingHost(),
aPointToInsert);
if (wsRunScannerAtInsertionPoint.GetEndReasonContent() &&
wsRunScannerAtInsertionPoint.GetEndReasonContent()->IsHTMLElement(
nsGkAtoms::br) &&
!HTMLEditUtils::IsVisibleBRElement(
*wsRunScannerAtInsertionPoint.EndReasonBRElementPtr(),
HTMLEditUtils::IsInvisibleBRElement(
*wsRunScannerAtInsertionPoint.GetEndReasonContent(),
wsRunScannerAtInsertionPoint.GetEditingHost())) {
return wsRunScannerAtInsertionPoint.EndReasonBRElementPtr();
}
@ -406,7 +404,7 @@ HTMLEditor::HTMLWithContextInserter::GetNewCaretPointAfterInsertingHTML(
if (wsRunScannerAtCaret
.ScanPreviousVisibleNodeOrBlockBoundaryFrom(pointToPutCaret)
.ReachedBRElement() &&
!HTMLEditUtils::IsVisibleBRElement(
HTMLEditUtils::IsInvisibleBRElement(
*wsRunScannerAtCaret.GetStartReasonContent(), editingHost)) {
WSRunScanner wsRunScannerAtStartReason(
editingHost,

Просмотреть файл

@ -1293,7 +1293,7 @@ nsresult HTMLEditor::AutoDeleteRangesHandler::ComputeRangesToDelete(
*scanFromCaretPointResult.BRElementPtr(), EditorType::HTML)) {
return NS_SUCCESS_DOM_NO_OPERATION;
}
if (!HTMLEditUtils::IsVisibleBRElement(
if (HTMLEditUtils::IsInvisibleBRElement(
*scanFromCaretPointResult.BRElementPtr(), editingHost)) {
EditorDOMPoint newCaretPosition =
aDirectionAndAmount == nsIEditor::eNext
@ -1567,7 +1567,7 @@ EditActionResult HTMLEditor::AutoDeleteRangesHandler::Run(
*scanFromCaretPointResult.BRElementPtr(), EditorType::HTML)) {
return EditActionCanceled();
}
if (!HTMLEditUtils::IsVisibleBRElement(
if (HTMLEditUtils::IsInvisibleBRElement(
*scanFromCaretPointResult.BRElementPtr(), editingHost)) {
// TODO: We should extend the range to delete again before/after
// the caret point and use `HandleDeleteNonCollapsedRanges()`
@ -1620,7 +1620,7 @@ EditActionResult HTMLEditor::AutoDeleteRangesHandler::Run(
return EditActionResult(NS_ERROR_FAILURE);
}
if (scanFromCaretPointResult.ReachedBRElement() &&
!HTMLEditUtils::IsVisibleBRElement(
HTMLEditUtils::IsInvisibleBRElement(
*scanFromCaretPointResult.BRElementPtr(), editingHost)) {
return EditActionHandled(NS_ERROR_EDITOR_UNEXPECTED_DOM_TREE);
}
@ -2387,9 +2387,8 @@ EditActionResult HTMLEditor::AutoDeleteRangesHandler::HandleDeleteAtomicContent(
const EditorDOMPoint& aCaretPoint,
const WSRunScanner& aWSRunScannerAtCaret) {
MOZ_ASSERT(aHTMLEditor.IsEditActionDataAvailable());
MOZ_ASSERT_IF(aAtomicContent.IsHTMLElement(nsGkAtoms::br),
HTMLEditUtils::IsVisibleBRElement(
aAtomicContent, aWSRunScannerAtCaret.GetEditingHost()));
MOZ_ASSERT(!HTMLEditUtils::IsInvisibleBRElement(
aAtomicContent, aWSRunScannerAtCaret.GetEditingHost()));
MOZ_ASSERT(&aAtomicContent != aWSRunScannerAtCaret.GetEditingHost());
nsresult rv =
@ -3373,8 +3372,7 @@ bool HTMLEditor::AutoDeleteRangesHandler::AutoBlockElementsJoiner::
{EmptyCheckOption::TreatSingleBRElementAsVisible})) {
continue;
}
if (!content->IsHTMLElement(nsGkAtoms::br) ||
HTMLEditUtils::IsVisibleBRElement(*content)) {
if (!HTMLEditUtils::IsInvisibleBRElement(*content)) {
return false;
}
}
@ -4434,8 +4432,7 @@ nsresult HTMLEditor::AutoDeleteRangesHandler::AutoBlockElementsJoiner::
NS_WARNING_ASSERTION(editingHost, "There was no editing host");
nsIContent* nextContent =
atStart.IsEndOfContainer() && range.StartRef().GetChild() &&
range.StartRef().GetChild()->IsHTMLElement(nsGkAtoms::br) &&
!HTMLEditUtils::IsVisibleBRElement(
HTMLEditUtils::IsInvisibleBRElement(
*range.StartRef().GetChild(), editingHost)
? HTMLEditUtils::GetNextContent(
*atStart.ContainerAsContent(),