Closed Bug 1861603 Opened 11 months ago Closed 11 months ago

Make `HTMLEditUtils::IsEmptyNode` refer computed style

Categories

(Core :: DOM: Editor, defect, P2)

defect

Tracking

()

RESOLVED FIXED
121 Branch
Tracking Status
firefox121 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

Attachments

(1 file)

HTMLEditUtils::IsEmptyNode checks the visibility of given node without too strict checks. Now, the callers can set the options to TreatListItemAsVisible and TreatTableCellAsVisible, but the method does not check the actual style of found elements. Therefore, e.g., HTMLEditUtils::IsEmptyInlineContainer may get odd result because it's defined as:

    return HTMLEditUtils::IsInlineContent(aContent, aBlockInlineCheck) &&
           HTMLEditUtils::IsContainerNode(aContent) &&
           HTMLEditUtils::IsEmptyNode(aContent, aOptions);

but only the first line refers the style.

See Also: → 1858794

Gecko starts referring computed style to consider whether block or inline and
HTMLEditUtils::IsEmptyNode is used with them to check a node like these:
https://searchfox.org/mozilla-central/rev/40d51bef58b8e901d6ab4b60dd280f372a0e417d/editor/libeditor/HTMLEditUtils.h#543,554

However, IsEmptyNode still checks only the tag names when the caller wants
to treat "list items" and/or "table cells" are visible (not empty). Therefore,
this mismatch may cause the assertion failure reported in the bug.

From code design point of view, HTMLEditUtils::IsListItem and
HTMLEditUtils::IsTableCell should refer the computed style. However, the
former is mostly used for checking the logical structure of HTML, and in the
latter case, we should keep the table structure even if table cells are styled
as inline because browsers do not support table structure recreation except
the Gecko specific editing UI (called inline table editor). Therefore, it does
not make sense that we would change them.

Instead, we should change IsEmptyNode which is used only for visibility check.
So, using computed style is always reasonable.

Note that this patch changes the behavior in some edge cases. The method always
treat a list item or a table cell which contains a sub-list or a sub table.
However, I would not like to add new EmptyCheckOption for it to check the
complicated style check. Therefore, this patch changes the behavior to treat
them visible if and only if they have another list item or another table cell.
Basically, <table> and list elements should have a least one table cell or
one list item. Therefore, this change should not appear in the web apps in the
wild.

Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/94d53eed0617 Make `HTMLEditUtils::IsEmptyNode` refer the computed style r=m_kato
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/43038 for changes under testing/web-platform/tests
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → 121 Branch
Upstream PR merged by moz-wptsync-bot
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: