Skip to content

Commit

Permalink
[cleanup] Remove EditingNG flag.
Browse files Browse the repository at this point in the history
As above.
There should be no behaviour change.

Change-Id: Icbfd57a05b87c48cbf72b3119b13a128f92b1da0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3561499
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988562}
  • Loading branch information
bfgeek authored and Chromium LUCI CQ committed Apr 4, 2022
1 parent b2d7809 commit 85a3263
Show file tree
Hide file tree
Showing 7 changed files with 10 additions and 181 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -604,12 +604,6 @@ static void AdjustStyleForDisplay(ComputedStyle& style,
style.UpdateFontOrientation();
}

// Disable editing custom layout elements, until EditingNG is ready.
if (!RuntimeEnabledFeatures::EditingNGEnabled() &&
(style.Display() == EDisplay::kLayoutCustom ||
style.Display() == EDisplay::kInlineLayoutCustom))
style.SetUserModify(EUserModify::kReadOnly);

if (layout_parent_style.IsDisplayFlexibleOrGridBox()) {
// We want to count vertical percentage paddings/margins on flex items
// because our current behavior is different from the spec and we want to
Expand Down
10 changes: 0 additions & 10 deletions third_party/blink/renderer/core/dom/element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -424,16 +424,6 @@ bool CalculateStyleShouldForceLegacyLayout(const Element& element,
if (style.DisplayTypeRequiresLayoutNG())
return false;

// TODO(layout-dev): Once LayoutNG handles inline content editable, we
// should get rid of following code fragment.
if (!RuntimeEnabledFeatures::EditingNGEnabled()) {
if (style.UsedUserModify() != EUserModify::kReadOnly ||
document.InDesignMode()) {
UseCounter::Count(document, WebFeature::kLegacyLayoutByEditing);
return true;
}
}

if (!RuntimeEnabledFeatures::LayoutNGBlockFragmentationEnabled()) {
// Disable NG for the entire subtree if we're establishing a multicol
// container.
Expand Down
21 changes: 0 additions & 21 deletions third_party/blink/renderer/core/editing/local_caret_rect_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1138,9 +1138,6 @@ TEST_P(ParameterizedLocalCaretRectTest, AfterCollapsedWhiteSpaceInRTLText) {

// https://crbug.com/936988
TEST_P(ParameterizedLocalCaretRectTest, AfterIneditableInline) {
// For LayoutNG, we also enable EditingNG to test NG caret rendering.
ScopedEditingNGForTest editing_ng(LayoutNGEnabled());

LoadAhem();
InsertStyleElement("div { font: 10px/10px Ahem }");
SetBodyContent(
Expand All @@ -1155,9 +1152,6 @@ TEST_P(ParameterizedLocalCaretRectTest, AfterIneditableInline) {

// http://crbug.com/688015
TEST_P(ParameterizedLocalCaretRectTest, LocalCaretAtBeginningOfNonEditable) {
// For LayoutNG, we also enable EditingNG to test NG caret rendering.
ScopedEditingNGForTest editing_ng(LayoutNGEnabled());

LoadAhem();
InsertStyleElement(
"div { width: 70px; padding-left: 10px; font: 10px/10px Ahem }"
Expand All @@ -1177,9 +1171,6 @@ TEST_P(ParameterizedLocalCaretRectTest, LocalCaretAtBeginningOfNonEditable) {
// http://crbug.com/688015
TEST_P(ParameterizedLocalCaretRectTest,
LocalCaretAtBeginningOfNonEditableInFlatTree) {
// For LayoutNG, we also enable EditingNG to test NG caret rendering.
ScopedEditingNGForTest editing_ng(LayoutNGEnabled());

LoadAhem();
InsertStyleElement(
"div { width: 70px; padding-left: 10px; font: 10px/10px Ahem }"
Expand Down Expand Up @@ -1217,9 +1208,6 @@ TEST_P(ParameterizedLocalCaretRectTest,

// http://crbug.com/688015
TEST_P(ParameterizedLocalCaretRectTest, LocalCaretAtEndOfNonEditable) {
// For LayoutNG, we also enable EditingNG to test NG caret rendering.
ScopedEditingNGForTest editing_ng(LayoutNGEnabled());

LoadAhem();
InsertStyleElement(
"div { width: 70px; padding: 10px; font: 10px/10px Ahem }"
Expand All @@ -1244,9 +1232,6 @@ TEST_P(ParameterizedLocalCaretRectTest, LocalCaretAtEndOfNonEditable) {
// http://crbug.com/688015
TEST_P(ParameterizedLocalCaretRectTest,
LocalCaretAtEndOfNonEditableInFlatTree) {
// For LayoutNG, we also enable EditingNG to test NG caret rendering.
ScopedEditingNGForTest editing_ng(LayoutNGEnabled());

LoadAhem();
InsertStyleElement(
"div { width: 70px; padding: 10px; font: 10px/10px Ahem }"
Expand Down Expand Up @@ -1296,9 +1281,6 @@ TEST_P(ParameterizedLocalCaretRectTest,

// http://crbug.com/688015
TEST_P(ParameterizedLocalCaretRectTest, AbsoluteCaretAtEndOfNonEditable) {
// For LayoutNG, we also enable EditingNG to test NG caret rendering.
ScopedEditingNGForTest editing_ng(LayoutNGEnabled());

LoadAhem();
InsertStyleElement(
"body { margin: 5px; }"
Expand Down Expand Up @@ -1327,9 +1309,6 @@ TEST_P(ParameterizedLocalCaretRectTest, AbsoluteCaretAtEndOfNonEditable) {

// http://crbug.com/688015
TEST_P(ParameterizedLocalCaretRectTest, AbsoluteCaretAtBeginningOfNonEditable) {
// For LayoutNG, we also enable EditingNG to test NG caret rendering.
ScopedEditingNGForTest editing_ng(LayoutNGEnabled());

LoadAhem();
InsertStyleElement(
"body { margin: 5px; }"
Expand Down
86 changes: 0 additions & 86 deletions third_party/blink/renderer/core/layout/force_legacy_layout_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,94 +21,8 @@ class ForceLegacyLayoutTest : public RenderingTest {
protected:
ForceLegacyLayoutTest()
: RenderingTest(MakeGarbageCollected<SingleChildLocalFrameClient>()) {}

bool EditingNGEnabled() { return RuntimeEnabledFeatures::EditingNGEnabled(); }
};

TEST_F(ForceLegacyLayoutTest, ForceLegacyBfcRecalcAncestorStyle) {
if (!RuntimeEnabledFeatures::LayoutNGEnabled())
return;

// This test assumes that contenteditable forces the entire block formatting
// context to use legacy layout. This will eventually change (when
// contenteditable is natively supported in LayoutNG), and when that happens,
// we'll need to come up with a different test here (provided that we're going
// to keep the legacy-forcing machinery for other reasons than
// contenteditable).
GetDocument().SetBaseURLOverride(KURL("http://test.com"));
SetBodyInnerHTML(R"HTML(
<div id="bfc" style="overflow:hidden;">
<div id="container" style="display:list-item;">
<div id="middle">
<div id="inner" style="overflow:hidden;">
<div id="child"></div>
</div>
</div>
</div>
</div>
)HTML");
UpdateAllLifecyclePhasesForTest();

Element* body = GetDocument().body();
Element* bfc = GetDocument().getElementById("bfc");
Element* container = GetDocument().getElementById("container");
Element* middle = GetDocument().getElementById("middle");
Element* inner = GetDocument().getElementById("inner");
Element* child = GetDocument().getElementById("child");

// Initially, everything should be laid out by NG.
EXPECT_TRUE(UsesNGLayout(*body));
EXPECT_TRUE(UsesNGLayout(*bfc));
EXPECT_TRUE(UsesNGLayout(*container));
EXPECT_TRUE(UsesNGLayout(*middle));
EXPECT_TRUE(UsesNGLayout(*inner));
EXPECT_TRUE(UsesNGLayout(*child));

// Enable contenteditable on an element that establishes a formatting context.
inner->setAttribute(html_names::kContenteditableAttr, "true");
UpdateAllLifecyclePhasesForTest();
EXPECT_TRUE(UsesNGLayout(*body));
EXPECT_TRUE(UsesNGLayout(*bfc));
EXPECT_TRUE(UsesNGLayout(*container));
EXPECT_TRUE(UsesNGLayout(*middle));
EXPECT_EQ(UsesNGLayout(*inner), EditingNGEnabled());
EXPECT_EQ(UsesNGLayout(*child), EditingNGEnabled());

// Remove overflow:hidden, so that the contenteditable element no longer
// establishes a formatting context.
inner->removeAttribute(html_names::kStyleAttr);
UpdateAllLifecyclePhasesForTest();
EXPECT_TRUE(UsesNGLayout(*body));
EXPECT_EQ(UsesNGLayout(*bfc), EditingNGEnabled());
EXPECT_EQ(UsesNGLayout(*container), EditingNGEnabled());
EXPECT_EQ(UsesNGLayout(*middle), EditingNGEnabled());
EXPECT_EQ(UsesNGLayout(*inner), EditingNGEnabled());
EXPECT_EQ(UsesNGLayout(*child), EditingNGEnabled());

// Change a non-inherited property. Legacy layout is triggered by #inner, but
// should be propagated all the way up to #container (which is the node that
// establishes the formatting context (due to overflow:hidden)). We'll now
// test that this persists through style recalculation.
middle->setAttribute(html_names::kStyleAttr, "background-color:blue;");
UpdateAllLifecyclePhasesForTest();
EXPECT_TRUE(UsesNGLayout(*body));
EXPECT_EQ(UsesNGLayout(*bfc), EditingNGEnabled());
EXPECT_EQ(UsesNGLayout(*container), EditingNGEnabled());
EXPECT_EQ(UsesNGLayout(*middle), EditingNGEnabled());
EXPECT_EQ(UsesNGLayout(*inner), EditingNGEnabled());
EXPECT_EQ(UsesNGLayout(*child), EditingNGEnabled());

// Change a property that requires re-attachment.
container->setAttribute(html_names::kStyleAttr, "display:block;");
UpdateAllLifecyclePhasesForTest();
EXPECT_TRUE(UsesNGLayout(*body));
EXPECT_EQ(UsesNGLayout(*bfc), EditingNGEnabled());
EXPECT_EQ(UsesNGLayout(*container), EditingNGEnabled());
EXPECT_EQ(UsesNGLayout(*middle), EditingNGEnabled());
EXPECT_EQ(UsesNGLayout(*inner), EditingNGEnabled());
EXPECT_EQ(UsesNGLayout(*child), EditingNGEnabled());
}

TEST_F(ForceLegacyLayoutTest, ForceLegacyMulticolSlot) {
if (!RuntimeEnabledFeatures::LayoutNGEnabled())
return;
Expand Down
33 changes: 0 additions & 33 deletions third_party/blink/renderer/core/layout/hit_testing_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -104,39 +104,6 @@ TEST_F(HitTestingTest, OcclusionHitTestWithClipPath) {
EXPECT_EQ(result.InnerNode(), occluder);
}

// crbug.com/1153037
TEST_F(HitTestingTest, LegacyInputElementInFragmentTraversal) {
ScopedLayoutNGFragmentTraversalForTest fragment_traversal_feature(true);
ScopedEditingNGForTest editing_feature(false);

SetBodyInnerHTML(R"HTML(
<style>
body { margin:100px; }
</style>
<input id="target">
)HTML");

const HitTestRequest hit_request(HitTestRequest::kActive);
const HitTestLocation hit_location(PhysicalOffset(110, 110));
HitTestResult hit_result(hit_request, hit_location);
ASSERT_TRUE(GetLayoutView().HitTest(hit_location, hit_result));
ASSERT_TRUE(hit_result.InnerNode());
const auto* layout_object = hit_result.InnerNode()->GetLayoutObject();
ASSERT_TRUE(layout_object);

// In this test we'll use the legacy layout engine for form controls, so the
// INPUT element will generate a LayoutTextControl with an inner editable
// LayoutBlockFlow child. We'll hit-test by traversing the fragment tree
// (rather than the LayoutObject tree). We should hit the inner
// LayoutBlockFlow. Since it is a legacy object and it is also laid out by a
// legacy parent, it will not generate any NG fragments. Check that we hit the
// right node, and that the hit-testing code hasn't incorrectly set an NG
// fragment from an ancestor.

ASSERT_EQ(layout_object->Parent()->GetNode(),
GetDocument().getElementById("target"));
}

TEST_F(HitTestingTest, ScrolledInline) {
SetBodyInnerHTML(R"HTML(
<style>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -976,11 +976,6 @@
name: "EditContext",
status: "experimental",
},
{
// http://crbug.com/707656 content editable in LayoutNG.
name: "EditingNG",
status: "stable",
},
{
name: "ElementSuperRareData",
status: "experimental",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,14 @@
<script src="../../resources/testharnessreport.js"></script>
<script src="../assert_selection.js"></script>
<script>
let supportsEditableMathML =
window.internals && internals.runtimeFlags.editingNGEnabled;

if (supportsEditableMathML) {
selection_test(
'<math id="target">a</math>',
selection => {
const target = selection.document.getElementById('target');
const style = selection.window.getComputedStyle(target);
supportsEditableMathML = style.display === 'math';
},
'<math id="target">a</math>',
'Check whether we have MathML or not');
}
selection_test(
'<math id="target">a</math>',
selection => {
const target = selection.document.getElementById('target');
const style = selection.window.getComputedStyle(target);
},
'<math id="target">a</math>',
'Check whether we have MathML or not');

function hasRedBackground(element) {
return window.getComputedStyle(element).
Expand Down Expand Up @@ -54,9 +48,7 @@
selection.document.execCommand('paste');
assert_false(hasRedBackground(document.documentElement));
},
supportsEditableMathML
? '<div contenteditable>teA<math>B<a style="display:block"><title>D<a id="</title><svg><style>*{background:red}</style>"></a></title></a></math>C|st</div>'
: '<div contenteditable>teA<math>B<br></math>C|<svg></svg>st</div>',
'<div contenteditable>teA<math>B<a style="display:block"><title>D<a id="</title><svg><style>*{background:red}</style>"></a></title></a></math>C|st</div>',
'Paste blocks SVG style injection');

// crbug.com/1141350
Expand All @@ -67,8 +59,6 @@
selection.document.execCommand('paste');
assert_equals(selection.document.querySelector('use'), null, 'SVG <use> with data URI should not leak into main document');
},
supportsEditableMathML
? '<div contenteditable><math><xss style="display:block"><xmp>X<a title="</xmp><div style=position:fixed;left:0;top:0;width:100%;height:100%><svg><use href=data:application/xml;base64,PHN2ZyBpZD0neCcgeG1sbnM9J2h0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnJz4KPGEgaHJlZj0namF2YXNjcmlwdDphbGVydCgxMjMpJz4KICAgIDxyZWN0IHdpZHRoPScxMDAlJyBoZWlnaHQ9JzEwMCUnIGZpbGw9J2xpZ2h0Ymx1ZScgLz4KICAgICA8dGV4dCB4PScwJyB5PScwJyBmaWxsPSdibGFjayc+CiAgICAgICA8dHNwYW4geD0nMCcgZHk9JzEuMmVtJz5Pb3BzLCB0aGVyZSdzIHNvbWV0aGluZyB3cm9uZyB3aXRoIHRoZSBwYWdlITwvdHNwYW4+CiAgICAgPHRzcGFuIHg9JzAnIGR5PScxLjJlbSc+UGxlYXNlIGNsaWNrIGhlcmUgdG8gcmVsb2FkLjwvdHNwYW4+Cjwvc3ZnPg==#x>">.<a></a></a></xmp></xss></math>t|abc</div>'
: '<div contenteditable>t<xmp>X&lt;a title="</xmp><div style="position: fixed; left: 0px; top: 0px; width: 800px; height: 600px;"><svg></svg></div>|abc</div>',
'<div contenteditable><math><xss style="display:block"><xmp>X<a title="</xmp><div style=position:fixed;left:0;top:0;width:100%;height:100%><svg><use href=data:application/xml;base64,PHN2ZyBpZD0neCcgeG1sbnM9J2h0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnJz4KPGEgaHJlZj0namF2YXNjcmlwdDphbGVydCgxMjMpJz4KICAgIDxyZWN0IHdpZHRoPScxMDAlJyBoZWlnaHQ9JzEwMCUnIGZpbGw9J2xpZ2h0Ymx1ZScgLz4KICAgICA8dGV4dCB4PScwJyB5PScwJyBmaWxsPSdibGFjayc+CiAgICAgICA8dHNwYW4geD0nMCcgZHk9JzEuMmVtJz5Pb3BzLCB0aGVyZSdzIHNvbWV0aGluZyB3cm9uZyB3aXRoIHRoZSBwYWdlITwvdHNwYW4+CiAgICAgPHRzcGFuIHg9JzAnIGR5PScxLjJlbSc+UGxlYXNlIGNsaWNrIGhlcmUgdG8gcmVsb2FkLjwvdHNwYW4+Cjwvc3ZnPg==#x>">.<a></a></a></xmp></xss></math>t|abc</div>',
'Paste blocks data URI in SVG use element injection via <math>');
</script>

0 comments on commit 85a3263

Please sign in to comment.