Some style changes cause tatechuyoko to be drawn off center

https://bugs.webkit.org/show_bug.cgi?id=150986 <rdar://problem/20748013> Reviewed by Darin Adler. Source/WebCore: Layouts should be idempotent. In particular, during layout, an element should not rely on a previous call to styleDidChange() with a sufficiently high StyleDifference. RenderCombineText was assuming that, if a layout occurs, a previous call to styleDidChange() would have reset the renderedText. However, an ancestor element might cause the RenderCombineText to re-combine when it is already combined. Therefore, the recombination should fully uncombine before recombining. Test: fast/text/text-combine-style-change-extra-layout.html * rendering/RenderCombineText.cpp: (WebCore::RenderCombineText::combineText): Fully uncombine before recombining. LayoutTests: * fast/text/text-combine-style-change-extra-layout-expected.html: Added. * fast/text/text-combine-style-change-extra-layout.html: Added. Canonical link: https://commits.webkit.org/169246@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@192169 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Nov 9, 2015 · f8e9e25 · f8e9e25
1 parent 4c30c6e
commit f8e9e25
Show file tree

Hide file tree

Showing 5 changed files with 71 additions and 2 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
+2015-11-09  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Some style changes cause tatechuyoko to be drawn off center
+        https://bugs.webkit.org/show_bug.cgi?id=150986
+        <rdar://problem/20748013>
+
+        Reviewed by Darin Adler.
+
+        * fast/text/text-combine-style-change-extra-layout-expected.html: Added.
+        * fast/text/text-combine-style-change-extra-layout.html: Added.
+
 2015-11-09  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         REGRESSION (r190883): Error calculating the tile size for an SVG with no intrinsic size but with large floating intrinsic ratio

diff --git a/LayoutTests/fast/text/text-combine-style-change-extra-layout-expected.html b/LayoutTests/fast/text/text-combine-style-change-extra-layout-expected.html
@@ -0,0 +1,12 @@
+<html>
+    <head>
+        <style>
+        #w {
+            background-color: green;
+        }
+        </style>
+    </head>
+    <body style="-webkit-writing-mode: vertical-rl; font-size: 320px;">
+        <span id="w" style="-webkit-text-combine: horizontal;">39</span>
+    </body>
+</html>
diff --git a/LayoutTests/fast/text/text-combine-style-change-extra-layout.html b/LayoutTests/fast/text/text-combine-style-change-extra-layout.html
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <style>
+            .asdf {
+                background-color: green;
+            }
+        </style>
+    </head>
+    <body style="-webkit-writing-mode: vertical-rl; font-size: 320px;">
+        <span id="w" style="-webkit-text-combine: horizontal;">39</span>
+		<script>
+            if (window.testRunner)
+                testRunner.waitUntilDone();
+            window.setTimeout(function() {
+                document.getElementById("w").className = "asdf";
+                if (window.testRunner)
+                    testRunner.notifyDone();
+            }, 0);
+        </script>
+    </body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,23 @@
+2015-11-09  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Some style changes cause tatechuyoko to be drawn off center
+        https://bugs.webkit.org/show_bug.cgi?id=150986
+        <rdar://problem/20748013>
+
+        Reviewed by Darin Adler.
+
+        Layouts should be idempotent. In particular, during layout, an element should not
+        rely on a previous call to styleDidChange() with a sufficiently high StyleDifference.
+        RenderCombineText was assuming that, if a layout occurs, a previous call to
+        styleDidChange() would have reset the renderedText. However, an ancestor element might
+        cause the RenderCombineText to re-combine when it is already combined. Therefore, the
+        recombination should fully uncombine before recombining.
+
+        Test: fast/text/text-combine-style-change-extra-layout.html
+
+        * rendering/RenderCombineText.cpp:
+        (WebCore::RenderCombineText::combineText): Fully uncombine before recombining.
+
 2015-11-09  Brent Fulgham  <bfulgham@apple.com>
 
         [Win] Recognize context flush as an event that requires an update

diff --git a/Source/WebCore/rendering/RenderCombineText.cpp b/Source/WebCore/rendering/RenderCombineText.cpp
@@ -90,6 +90,10 @@ void RenderCombineText::combineText()
     if (!m_needsFontUpdate)
         return;
 
+    // An ancestor element may trigger us to lay out again, even when we're already combined.
+    if (m_isCombined)
+        RenderText::setRenderedText(originalText());
+
     m_isCombined = false;
     m_needsFontUpdate = false;
 
@@ -141,8 +145,8 @@ void RenderCombineText::combineText()
         m_combineFontStyle->fontCascade().update(fontSelector);
 
     if (m_isCombined) {
-        DEPRECATED_DEFINE_STATIC_LOCAL(String, objectReplacementCharacterString, (&objectReplacementCharacter, 1));
-        RenderText::setRenderedText(objectReplacementCharacterString.impl());
+        static NeverDestroyed<String> objectReplacementCharacterString(&objectReplacementCharacter, 1);
+        RenderText::setRenderedText(objectReplacementCharacterString.get());
         m_combinedTextSize = FloatSize(combinedTextWidth, glyphOverflow.bottom + glyphOverflow.top);
     }
 }