commit bdc0aa0314a69f9b24e5de7112b10ee86bb3634f
parent 91d2164cd7426b3def4dd0cbe0e25a6acf7d5faa
Author: Masayuki Nakano <masayuki@d-toybox.com>
Date: Mon, 17 Nov 2025 23:53:08 +0000
Bug 2000127 - Rewrite `HTMLEditUtils::IsFlexOrGridItem()` again and make `HTMLEditUtils::IsBlockElement()` return true for the items r=m_kato
When a child of flex or grid container's `display` is `contents`, its
children become the flex/grid items. Then, `<display-inside>` of
the children is computed as `flex` or `grid`, but the
`<display-outside>` may be `inline`. For the consistency with normal
flex/grid items, such "inherited" items should also be treated as
blocks. Therefore, we need to make `HTMLEditUtils::IsBlockElement` and
`HTMLEditUtils::IsInlineContent` check whether the given element is a
flex/grid item.
Differential Revision: https://phabricator.services.mozilla.com/D272582
Diffstat:
7 files changed, 196 insertions(+), 22 deletions(-)
diff --git a/editor/libeditor/HTMLEditUtils.cpp b/editor/libeditor/HTMLEditUtils.cpp
@@ -279,6 +279,8 @@ bool HTMLEditUtils::IsBlockElement(const nsIContent& aContent,
MOZ_ASSERT(aBlockInlineCheck != BlockInlineCheck::Auto);
if (MOZ_UNLIKELY(!aContent.IsElement())) {
+ // FIXME: If aContent is a visible `Text` and a flex/grid item, we should
+ // treat it as block.
return false;
}
// If it's a <br>, we should always treat it as an inline element because
@@ -313,6 +315,11 @@ bool HTMLEditUtils::IsBlockElement(const nsIContent& aContent,
if (!styleDisplay->IsInlineOutsideStyle()) {
return true;
}
+ // Special case. If aContent is a grid or flex item, we want to treat it as a
+ // block to handle it with the general paths.
+ if (HTMLEditUtils::ParentElementIsGridOrFlexContainer(aContent)) {
+ return true;
+ }
// If we're checking display-inside, inline-block, etc should be a block too.
return aBlockInlineCheck == BlockInlineCheck::UseComputedDisplayStyle &&
styleDisplay->DisplayInside() == StyleDisplayInside::FlowRoot &&
@@ -327,6 +334,8 @@ bool HTMLEditUtils::IsInlineContent(const nsIContent& aContent,
MOZ_ASSERT(aBlockInlineCheck != BlockInlineCheck::Auto);
if (!aContent.IsElement()) {
+ // FIXME: If aContent is a visible `Text` and a flex/grid item, we should
+ // treat it as block.
return true;
}
// If it's a <br>, we should always treat it as an inline element because
@@ -357,13 +366,28 @@ bool HTMLEditUtils::IsInlineContent(const nsIContent& aContent,
// style if aContent.
return !IsHTMLBlockElementByDefault(aContent);
}
+ // Special case. If aContent is a grid or flex item, we want to treat it as a
+ // block to handle it with the general paths.
+ if (HTMLEditUtils::ParentElementIsGridOrFlexContainer(aContent)) {
+ return false;
+ }
// Different block IsBlockElement, when the display-outside is inline, it's
// simply an inline element.
return styleDisplay->IsInlineOutsideStyle();
}
-bool HTMLEditUtils::IsFlexOrGridItem(const Element& aElement) {
- Element* const parentElement = aElement.GetParentElement();
+bool HTMLEditUtils::ParentElementIsGridOrFlexContainer(
+ const nsIContent& aMaybeFlexOrGridItemContent) {
+ if (!aMaybeFlexOrGridItemContent.IsElement()) {
+ if (!aMaybeFlexOrGridItemContent.IsText() ||
+ !aMaybeFlexOrGridItemContent.AsText()->TextDataLength()) {
+ return false;
+ }
+ // FIXME: If aMaybeFlexOrGridItemContent has only collapsible white-spaces
+ // and next to a block boundary, it's invisible and shouldn't be a flex/grid
+ // item. However, scanning block boundary requires to call this method.
+ }
+ Element* const parentElement = aMaybeFlexOrGridItemContent.GetParentElement();
// Editable state does not affect to elements across shadow DOM boundaries.
// Therefore, we don't need to treat aElement as an flex item nor a grid item
// as so if aElement is a root element of a shadow DOM unless we'll support
@@ -376,8 +400,11 @@ bool HTMLEditUtils::IsFlexOrGridItem(const Element& aElement) {
// We should consider whether the element is a flex item or a grid item
// without nsIFrame since we don't want to refresh the layout while
// `HTMLEditor` handles an action to avoid to run script.
- RefPtr<const ComputedStyle> elementStyle =
- nsComputedDOMStyle::GetComputedStyleNoFlush(&aElement);
+ const RefPtr<const ComputedStyle> elementStyle =
+ nsComputedDOMStyle::GetComputedStyleNoFlush(
+ aMaybeFlexOrGridItemContent.IsElement()
+ ? aMaybeFlexOrGridItemContent.AsElement()
+ : parentElement);
if (MOZ_UNLIKELY(!elementStyle)) {
return false;
}
@@ -386,23 +413,30 @@ bool HTMLEditUtils::IsFlexOrGridItem(const Element& aElement) {
return false;
}
const RefPtr<const ComputedStyle> parentElementStyle =
- nsComputedDOMStyle::GetComputedStyleNoFlush(parentElement);
+ aMaybeFlexOrGridItemContent.IsElement()
+ ? nsComputedDOMStyle::GetComputedStyleNoFlush(parentElement)
+ : elementStyle;
if (MOZ_UNLIKELY(!parentElementStyle)) {
return false;
}
- const nsStyleDisplay* parentStyleDisplay = parentElementStyle->StyleDisplay();
- const auto parentDisplayInside = parentStyleDisplay->DisplayInside();
- const bool parentIsFlexContainerOrGridContainer =
- parentDisplayInside == StyleDisplayInside::Flex ||
- parentDisplayInside == StyleDisplayInside::Grid;
- // We assume that the computed `display` value of a flex item and a grid item
- // is "block". If it would be false, we need to update
- // HTMLEditUtils::IsBlockElement() and HTMLEditUtils::IsInlineContent().
- // However, they are called a lot so that we need to be careful about changing
- // them to check the parent style.
- MOZ_ASSERT_IF(parentIsFlexContainerOrGridContainer,
- styleDisplay->IsBlockOutsideStyle());
- return parentIsFlexContainerOrGridContainer;
+ const auto parentDisplayInside =
+ parentElementStyle->StyleDisplay()->DisplayInside();
+ return parentDisplayInside == StyleDisplayInside::Flex ||
+ parentDisplayInside == StyleDisplayInside::Grid;
+}
+
+bool HTMLEditUtils::IsFlexOrGridItem(const nsIContent& aContent) {
+ if (!HTMLEditUtils::ParentElementIsGridOrFlexContainer(aContent)) {
+ return false;
+ }
+ // Note that if parent element's `display` is `contents`, the
+ // `display-outside` style of aElement may not be block. However, even in
+ // such case, HTMLEditUtils::IsBlockElement() should return true.
+ MOZ_ASSERT_IF(aContent.IsElement(),
+ HTMLEditUtils::IsBlockElement(
+ *aContent.AsElement(),
+ BlockInlineCheck::UseComputedDisplayOutsideStyle));
+ return true;
}
bool HTMLEditUtils::IsInclusiveAncestorCSSDisplayNone(
diff --git a/editor/libeditor/HTMLEditUtils.h b/editor/libeditor/HTMLEditUtils.h
@@ -233,10 +233,11 @@ class HTMLEditUtils final {
[[nodiscard]] static bool IsDisplayInsideFlowRoot(const Element& aElement);
/**
- * Return true if aElement is a flex item or a grid item. This works only
- * when aElement has a primary frame.
+ * Return true if aContent is a flex item or a grid item. Note that if
+ * aContent is the `Text` node in the following case, this returns `true`.
+ * <div style="display:flex"><span style="display:contents">text</span></div>
*/
- [[nodiscard]] static bool IsFlexOrGridItem(const Element& aElement);
+ [[nodiscard]] static bool IsFlexOrGridItem(const nsIContent& aContent);
/**
* IsRemovableInlineStyleElement() returns true if aElement is an inline
@@ -3214,6 +3215,15 @@ class HTMLEditUtils final {
*/
static Element* GetElementOfImmediateBlockBoundary(
const nsIContent& aContent, const WalkTreeDirection aDirection);
+
+ /**
+ * Return true if parent element is a grid or flex container.
+ * Note that even if the parent is a grid/flex container, the
+ * <display-outside> of aMaybeFlexOrGridItemContent may be "inline" if the
+ * parent is also a grid/flex item but has `display:contents`.
+ */
+ [[nodiscard]] static bool ParentElementIsGridOrFlexContainer(
+ const nsIContent& aMaybeFlexOrGridItemContent);
};
/**
diff --git a/testing/web-platform/meta/editing/run/delete.html.ini b/testing/web-platform/meta/editing/run/delete.html.ini
@@ -657,3 +657,9 @@
[[["delete",""\]\] "<div style=display:inline-grid><span>{}<br></span></div><div>abc</div>" compare innerHTML]
expected: FAIL
+
+ [[["delete",""\]\] "<div style=display:flex><span>abc</span><span style=display:contents><span>[\]def</span></span></div>" compare innerHTML]
+ expected: FAIL
+
+ [[["delete",""\]\] "<div style=display:grid><span>abc</span><span style=display:contents><span>[\]def</span></span></div>" compare innerHTML]
+ expected: FAIL
diff --git a/testing/web-platform/meta/editing/run/forwarddelete.html.ini b/testing/web-platform/meta/editing/run/forwarddelete.html.ini
@@ -568,3 +568,9 @@
[[["forwarddelete",""\]\] "<div>abc</div><div style=display:inline-grid><span>{}<br></span></div>" compare innerHTML]
expected: FAIL
+
+ [[["forwarddelete",""\]\] "<div style=display:flex><span>abc[\]</span><span style=display:contents><span>def</span></span></div>" compare innerHTML]
+ expected: FAIL
+
+ [[["forwarddelete",""\]\] "<div style=display:grid><span>abc[\]</span><span style=display:contents><span>def</span></span></div>" compare innerHTML]
+ expected: FAIL
diff --git a/testing/web-platform/tests/editing/crashtests/inserttext-to-delete-flex-item-boundary-whose-display-contents.html b/testing/web-platform/tests/editing/crashtests/inserttext-to-delete-flex-item-boundary-whose-display-contents.html
@@ -0,0 +1,30 @@
+<!doctype html>
+<html>
+<head>
+<meta charset="utf-8">
+<script>
+"use strict";
+
+addEventListener("load", () => {
+ document.designMode = "on";
+ getSelection().setBaseAndExtent(
+ document.querySelector("b"),
+ 1,
+ document.querySelector("span"),
+ 1
+ );
+ document.execCommand("insertText", false, "A");
+}, {once: true});
+</script>
+</head>
+<body>
+<div style="display:inline-grid">
+ <b>
+ </b>
+ <i style="display:contents">
+ <span>
+ </span>
+ </i>
+</div>
+</body>
+</html>
diff --git a/testing/web-platform/tests/editing/data/delete.js b/testing/web-platform/tests/editing/data/delete.js
@@ -3400,4 +3400,48 @@ var browserTests = [
"<div><div>abc</div></div><div>def</div>"],
[true],
{}],
+
+// display:contents of grid/flex item makes its children as items of its container.
+// Therefore, they should be treated as items correctly, and the display:contents elements should
+// be treated as meaningless container. Therefore, they should be deleted once they become empty.
+["<div style=display:flex><span>abc</span><span style=display:contents>[]def</span></div>",
+ [["delete",""]],
+ "<div style=\"display:flex\"><span>abcdef</span></div>",
+ [true],
+ {}],
+["<div style=display:flex><span>abc</span><span style=display:contents><span>[]def</span></span></div>",
+ [["delete",""]],
+ "<div style=\"display:flex\"><span>abcdef</span></div>",
+ [true],
+ {}],
+["<div style=display:flex><span style=display:contents>abc</span><span>[]def</span></div>",
+ [["delete",""]],
+ "<div style=\"display:flex\"><span style=\"display:contents\">abcdef</span></div>",
+ [true],
+ {}],
+["<div style=display:flex><span style=display:contents><span>abc</span></span><span>[]def</span></div>",
+ [["delete",""]],
+ "<div style=\"display:flex\"><span style=\"display:contents\"><span>abcdef</span></span></div>",
+ [true],
+ {}],
+["<div style=display:grid><span>abc</span><span style=display:contents>[]def</span></div>",
+ [["delete",""]],
+ "<div style=\"display:grid\"><span>abcdef</span></div>",
+ [true],
+ {}],
+["<div style=display:grid><span>abc</span><span style=display:contents><span>[]def</span></span></div>",
+ [["delete",""]],
+ "<div style=\"display:grid\"><span>abcdef</span></div>",
+ [true],
+ {}],
+["<div style=display:grid><span style=display:contents>abc</span><span>[]def</span></div>",
+ [["delete",""]],
+ "<div style=\"display:grid\"><span style=\"display:contents\">abcdef</span></div>",
+ [true],
+ {}],
+["<div style=display:grid><span style=display:contents><span>abc</span></span><span>[]def</span></div>",
+ [["delete",""]],
+ "<div style=\"display:grid\"><span style=\"display:contents\"><span>abcdef</span></span></div>",
+ [true],
+ {}],
]
diff --git a/testing/web-platform/tests/editing/data/forwarddelete.js b/testing/web-platform/tests/editing/data/forwarddelete.js
@@ -3257,9 +3257,53 @@ var browserTests = [
{}],
// XXX I'm not sure which result is better.
["<div><div>abc</div>{}<br></div><div>def</div>",
- [["delete",""]],
+ [["forwarddelete",""]],
["<div><div>abc</div>def</div>",
"<div><div>abc</div></div><div>def</div>"],
[true],
{}],
+
+// display:contents of grid/flex item makes its children as items of its container.
+// Therefore, they should be treated as items correctly, and the display:contents elements should
+// be treated as meaningless container. Therefore, they should be deleted once they become empty.
+["<div style=display:flex><span>abc[]</span><span style=display:contents>def</span></div>",
+ [["forwarddelete",""]],
+ "<div style=\"display:flex\"><span>abcdef</span></div>",
+ [true],
+ {}],
+["<div style=display:flex><span>abc[]</span><span style=display:contents><span>def</span></span></div>",
+ [["forwarddelete",""]],
+ "<div style=\"display:flex\"><span>abcdef</span></div>",
+ [true],
+ {}],
+["<div style=display:flex><span style=display:contents>abc[]</span><span>def</span></div>",
+ [["forwarddelete",""]],
+ "<div style=\"display:flex\"><span style=\"display:contents\">abcdef</span></div>",
+ [true],
+ {}],
+["<div style=display:flex><span style=display:contents><span>abc[]</span></span><span>def</span></div>",
+ [["forwarddelete",""]],
+ "<div style=\"display:flex\"><span style=\"display:contents\"><span>abcdef</span></span></div>",
+ [true],
+ {}],
+["<div style=display:grid><span>abc[]</span><span style=display:contents>def</span></div>",
+ [["forwarddelete",""]],
+ "<div style=\"display:grid\"><span>abcdef</span></div>",
+ [true],
+ {}],
+["<div style=display:grid><span>abc[]</span><span style=display:contents><span>def</span></span></div>",
+ [["forwarddelete",""]],
+ "<div style=\"display:grid\"><span>abcdef</span></div>",
+ [true],
+ {}],
+["<div style=display:grid><span style=display:contents>abc[]</span><span>def</span></div>",
+ [["forwarddelete",""]],
+ "<div style=\"display:grid\"><span style=\"display:contents\">abcdef</span></div>",
+ [true],
+ {}],
+["<div style=display:grid><span style=display:contents><span>abc[]</span></span><span>def</span></div>",
+ [["forwarddelete",""]],
+ "<div style=\"display:grid\"><span style=\"display:contents\"><span>abcdef</span></span></div>",
+ [true],
+ {}],
]
