Overlapping text on all CSS fonts specs
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 Oct 2017 13:11:05 +0000 (13:11 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 Oct 2017 13:11:05 +0000 (13:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=177585
<rdar://problem/34704078>

Reviewed by Daniel Bates.

Source/WebCore:

We were resetting StyleResolver::overrideDocumentElementStyle too early when resolving slot elements.
This resulted in 'rem' units being miscomputed.

Reduction by Zalan.

Test: fast/html/details-line-height-overlap.html

* style/StyleTreeResolver.cpp:
(WebCore::Style::TreeResolver::Scope::Scope):
(WebCore::Style::TreeResolver::Scope::~Scope):

    Only reset overrideDocumentElementStyle when destroying the scope.

(WebCore::Style::TreeResolver::pushScope):
(WebCore::Style::TreeResolver::pushEnclosingScope):
(WebCore::Style::TreeResolver::popScope):

    A scope can show up multiple times in scope stack.

* style/StyleTreeResolver.h:

LayoutTests:

* fast/html/details-line-height-overlap-expected.html: Added.
* fast/html/details-line-height-overlap.html: Added.
* platform/ios/fast/shadow-dom/copy-shadow-tree-expected.txt:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@223688 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/html/details-line-height-overlap-expected.html [new file with mode: 0644]
LayoutTests/fast/html/details-line-height-overlap.html [new file with mode: 0644]
LayoutTests/platform/ios/fast/shadow-dom/copy-shadow-tree-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/css/StyleResolver.h
Source/WebCore/style/StyleTreeResolver.cpp
Source/WebCore/style/StyleTreeResolver.h

index 5801408..cf3d4a2 100644 (file)
@@ -1,3 +1,15 @@
+2017-10-19  Antti Koivisto  <antti@apple.com>
+
+        Overlapping text on all CSS fonts specs
+        https://bugs.webkit.org/show_bug.cgi?id=177585
+        <rdar://problem/34704078>
+
+        Reviewed by Daniel Bates.
+
+        * fast/html/details-line-height-overlap-expected.html: Added.
+        * fast/html/details-line-height-overlap.html: Added.
+        * platform/ios/fast/shadow-dom/copy-shadow-tree-expected.txt:
+
 2017-10-18  Ryosuke Niwa  <rniwa@webkit.org>
 
         Don't expose raw HTML in pasteboard to the web content
diff --git a/LayoutTests/fast/html/details-line-height-overlap-expected.html b/LayoutTests/fast/html/details-line-height-overlap-expected.html
new file mode 100644 (file)
index 0000000..9154aff
--- /dev/null
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+<style>
+html {
+    font-size: 15px;
+}
+li {
+    line-height: 15px;
+}
+</style>
+<details><summary>details</summary></details>
+<li>1 should not</li>
+<li>2 overlap</li>
diff --git a/LayoutTests/fast/html/details-line-height-overlap.html b/LayoutTests/fast/html/details-line-height-overlap.html
new file mode 100644 (file)
index 0000000..7155165
--- /dev/null
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+<style>
+html {
+    font-size: 15px;
+}
+li {
+       line-height: 1rem; 
+}
+</style>
+<details><summary>details</summary></details>
+<li>1 should not</li>
+<li>2 overlap</li>
index 18050c9..f84e424 100644 (file)
@@ -1,7 +1,7 @@
 layer at (0,0) size 800x600
   RenderView at (0,0) size 800x600
-layer at (0,0) size 800x140
-  RenderBlock {HTML} at (0,0) size 800x140
+layer at (0,0) size 800x148
+  RenderBlock {HTML} at (0,0) size 800x148
     RenderBody {BODY} at (8,16) size 784x116
       RenderBlock {P} at (0,0) size 784x40
         RenderText {#text} at (0,0) size 330x19
index d90502c..bfc459a 100644 (file)
@@ -1,3 +1,32 @@
+2017-10-19  Antti Koivisto  <antti@apple.com>
+
+        Overlapping text on all CSS fonts specs
+        https://bugs.webkit.org/show_bug.cgi?id=177585
+        <rdar://problem/34704078>
+
+        Reviewed by Daniel Bates.
+
+        We were resetting StyleResolver::overrideDocumentElementStyle too early when resolving slot elements.
+        This resulted in 'rem' units being miscomputed.
+
+        Reduction by Zalan.
+
+        Test: fast/html/details-line-height-overlap.html
+
+        * style/StyleTreeResolver.cpp:
+        (WebCore::Style::TreeResolver::Scope::Scope):
+        (WebCore::Style::TreeResolver::Scope::~Scope):
+
+            Only reset overrideDocumentElementStyle when destroying the scope.
+
+        (WebCore::Style::TreeResolver::pushScope):
+        (WebCore::Style::TreeResolver::pushEnclosingScope):
+        (WebCore::Style::TreeResolver::popScope):
+
+            A scope can show up multiple times in scope stack.
+
+        * style/StyleTreeResolver.h:
+
 2017-10-19  Ryosuke Niwa  <rniwa@webkit.org>
 
         Consolidate calls to insertedInto and expand the coverage of NoEventDispatchAssertion
index 38f94e6..0ded1d9 100644 (file)
@@ -153,6 +153,7 @@ public:
 
     const MediaQueryEvaluator& mediaQueryEvaluator() const { return m_mediaQueryEvaluator; }
 
+    RenderStyle* overrideDocumentElementStyle() const { return m_overrideDocumentElementStyle; }
     void setOverrideDocumentElementStyle(RenderStyle* style) { m_overrideDocumentElementStyle = style; }
 
     void addCurrentSVGFontFaceRules();
index 94ea2df..b6a61ce 100644 (file)
@@ -75,6 +75,12 @@ TreeResolver::Scope::Scope(ShadowRoot& shadowRoot, Scope& enclosingScope)
     , shadowRoot(&shadowRoot)
     , enclosingScope(&enclosingScope)
 {
+    styleResolver.setOverrideDocumentElementStyle(enclosingScope.styleResolver.overrideDocumentElementStyle());
+}
+
+TreeResolver::Scope::~Scope()
+{
+    styleResolver.setOverrideDocumentElementStyle(nullptr);
 }
 
 TreeResolver::Parent::Parent(Document& document)
@@ -93,19 +99,16 @@ TreeResolver::Parent::Parent(Element& element, const RenderStyle& style, Change
 void TreeResolver::pushScope(ShadowRoot& shadowRoot)
 {
     m_scopeStack.append(adoptRef(*new Scope(shadowRoot, scope())));
-    scope().styleResolver.setOverrideDocumentElementStyle(m_documentElementStyle.get());
 }
 
 void TreeResolver::pushEnclosingScope()
 {
     ASSERT(scope().enclosingScope);
     m_scopeStack.append(*scope().enclosingScope);
-    scope().styleResolver.setOverrideDocumentElementStyle(m_documentElementStyle.get());
 }
 
 void TreeResolver::popScope()
 {
-    scope().styleResolver.setOverrideDocumentElementStyle(nullptr);
     return m_scopeStack.removeLast();
 }
 
index eb0d18e..7dbb287 100644 (file)
@@ -68,6 +68,7 @@ private:
 
         Scope(Document&);
         Scope(ShadowRoot&, Scope& enclosingScope);
+        ~Scope();
     };
 
     struct Parent {