Changeset 224773 in webkit

Timestamp:

Nov 13, 2017 1:14:46 PM (6 years ago)

Author:

Antti Koivisto

Message:

Don't eliminate whitespace renderer if the previous sibling is a text renderer
​https://bugs.webkit.org/show_bug.cgi?id=179620

Reviewed by Zalan Bujtas.

Source/WebCore:

Currently whitespace elimination code doesn't consider runs of text renderers. We should always make whitespace
renderer if the previous renderer is a text renderer. The behavior should be the same as if those were a single
renderer with merged text. This situation can happen easily with display:contents.

This fixes the remaining flexbox failures in display:contents tests.

• style/RenderTreeUpdater.cpp:

(WebCore::RenderTreeUpdater::textRendererIsNeeded):

We need a renderer if the previous rendere is RenderText.

LayoutTests:

• TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/style/RenderTreeUpdater.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2017-11-13  Antti Koivisto  <antti@apple.com>
+
+        Don't eliminate whitespace renderer if the previous sibling is a text renderer
+        https://bugs.webkit.org/show_bug.cgi?id=179620
+
+        Reviewed by Zalan Bujtas.
+
+        * TestExpectations:
+
 2017-11-13  Chris Dumez  <cdumez@apple.com>
 

trunk/LayoutTests/TestExpectations

@@ -1379 +1379 @@
 ### START OF display: contents failures
 
-webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-flex-003.html [ ImageOnlyFailure ]
 webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-table-001-inline.html [ ImageOnlyFailure ]
-webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-flex-002-none.html [ ImageOnlyFailure ]
-webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-flex-002.html [ ImageOnlyFailure ]
 webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-flow-root-001.html [ ImageOnlyFailure ]
-webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-flex-003-none.html [ ImageOnlyFailure ]
-webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-flex-002-inline.html [ ImageOnlyFailure ]
-webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-flex-003-inline.html [ ImageOnlyFailure ]
 
 ### END OF display: contents failures

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-11-13  Antti Koivisto  <antti@apple.com>
+
+        Don't eliminate whitespace renderer if the previous sibling is a text renderer
+        https://bugs.webkit.org/show_bug.cgi?id=179620
+
+        Reviewed by Zalan Bujtas.
+
+        Currently whitespace elimination code doesn't consider runs of text renderers. We should always make whitespace
+        renderer if the previous renderer is a text renderer. The behavior should be the same as if those were a single
+        renderer with merged text. This situation can happen easily with display:contents.
+
+        This fixes the remaining flexbox failures in display:contents tests.
+
+        * style/RenderTreeUpdater.cpp:
+        (WebCore::RenderTreeUpdater::textRendererIsNeeded):
+
+        We need a renderer if the previous rendere is RenderText.
+
 2017-11-13  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/style/RenderTreeUpdater.cpp

@@ -399 +399 @@
 bool RenderTreeUpdater::textRendererIsNeeded(const Text& textNode)
 {
-    const RenderElement& parentRenderer = renderTreePosition().parent();
+    auto& renderingParent = this->renderingParent();
+    auto& parentRenderer = renderingParent.renderTreePosition->parent();
     if (!parentRenderer.canHaveChildren())
         return false;
@@ -410 +411 @@
     if (!textNode.containsOnlyWhitespace())
         return true;
+    if (is<RenderText>(renderingParent.previousChildRenderer))
+        return true;
     // This text node has nothing but white space. We may still need a renderer in some cases.
-    if (parentRenderer.isTable() || parentRenderer.isTableRow() || parentRenderer.isTableSection() || parentRenderer.isRenderTableCol() || parentRenderer.isFrameSet() || (parentRenderer.isFlexibleBox() && !parentRenderer.isRenderButton()))
+    if (parentRenderer.isTable() || parentRenderer.isTableRow() || parentRenderer.isTableSection() || parentRenderer.isRenderTableCol() || parentRenderer.isFrameSet())
+        return false;
+    if (parentRenderer.isFlexibleBox() && !parentRenderer.isRenderButton())
         return false;
     if (parentRenderer.style().preserveNewline()) // pre/pre-wrap/pre-line always make renderers.
         return true;
 
-    auto* previousRenderer = renderingParent().previousChildRenderer;
+    auto* previousRenderer = renderingParent.previousChildRenderer;
     if (previousRenderer && previousRenderer->isBR()) // <span><br/> <br/></span>
         return false;