Centering text inside a button set to display flex and justify-content: center is...
authorhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Feb 2017 21:47:38 +0000 (21:47 +0000)
committerhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Feb 2017 21:47:38 +0000 (21:47 +0000)
https://bugs.webkit.org/show_bug.cgi?id=148872
<rdar://problem/22590086>

Reviewed by Simon Fraser.

Source/WebCore:

Added new test in fast/forms.

* rendering/RenderButton.cpp:
(WebCore::RenderButton::addChild):
(WebCore::RenderButton::updateAnonymousChildStyle):
(WebCore::RenderButton::styleDidChange):
(WebCore::RenderButton::styleWillChange): Deleted.
(WebCore::RenderButton::setupInnerStyle): Deleted.
* rendering/RenderButton.h:
* rendering/RenderElement.cpp:
(WebCore::RenderElement::propagateStyleToAnonymousChildren):
* rendering/RenderElement.h:
(WebCore::RenderElement::updateAnonymousChildStyle):

LayoutTests:

* fast/forms/button-set-display-flex-justifyContent-center-expected.html: Added.
* fast/forms/button-set-display-flex-justifyContent-center.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/forms/button-set-display-flex-justifyContent-center-expected.html [new file with mode: 0644]
LayoutTests/fast/forms/button-set-display-flex-justifyContent-center.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderButton.cpp
Source/WebCore/rendering/RenderButton.h
Source/WebCore/rendering/RenderElement.cpp
Source/WebCore/rendering/RenderElement.h

index cc9aaee..bbb17c1 100644 (file)
@@ -1,3 +1,14 @@
+2017-02-28  Dave Hyatt  <hyatt@apple.com>
+
+        Centering text inside a button set to display flex and justify-content: center is impossible
+        https://bugs.webkit.org/show_bug.cgi?id=148872
+        <rdar://problem/22590086>
+
+        Reviewed by Simon Fraser.
+
+        * fast/forms/button-set-display-flex-justifyContent-center-expected.html: Added.
+        * fast/forms/button-set-display-flex-justifyContent-center.html: Added.
+
 2017-02-28  Chris Dumez  <cdumez@apple.com>
 
         [iOS] Throttle requestAnimationFrame to 30fps in low power mode
diff --git a/LayoutTests/fast/forms/button-set-display-flex-justifyContent-center-expected.html b/LayoutTests/fast/forms/button-set-display-flex-justifyContent-center-expected.html
new file mode 100644 (file)
index 0000000..ec7e9b5
--- /dev/null
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<style>
+div {
+    font-family: sans-serif;
+    font-size: initial;
+    width: 300px;
+    display: flex;
+    justify-content: center;
+}
+</style>
+<h4>Test for centering text inside a button set to display flex and justify-content: center is impossible</h4>
+<div>text with justify-content: center</div>
diff --git a/LayoutTests/fast/forms/button-set-display-flex-justifyContent-center.html b/LayoutTests/fast/forms/button-set-display-flex-justifyContent-center.html
new file mode 100644 (file)
index 0000000..3496322
--- /dev/null
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<style>
+button {
+    border: 0;
+    padding: 0;
+    background-color: white;
+    font-family: sans-serif;
+    font-size: initial;
+    width: 300px;
+    display: flex;
+    justify-content: center;
+}
+</style>
+<h4>Test for centering text inside a button set to display flex and justify-content: center is impossible</h4>
+<button>text with justify-content: center</button>
index aa1d257..20d6e5a 100644 (file)
@@ -1,3 +1,25 @@
+2017-02-28  Dave Hyatt  <hyatt@apple.com>
+
+        Centering text inside a button set to display flex and justify-content: center is impossible
+        https://bugs.webkit.org/show_bug.cgi?id=148872
+        <rdar://problem/22590086>
+
+        Reviewed by Simon Fraser.
+
+        Added new test in fast/forms.
+
+        * rendering/RenderButton.cpp:
+        (WebCore::RenderButton::addChild):
+        (WebCore::RenderButton::updateAnonymousChildStyle):
+        (WebCore::RenderButton::styleDidChange):
+        (WebCore::RenderButton::styleWillChange): Deleted.
+        (WebCore::RenderButton::setupInnerStyle): Deleted.
+        * rendering/RenderButton.h:
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::propagateStyleToAnonymousChildren):
+        * rendering/RenderElement.h:
+        (WebCore::RenderElement::updateAnonymousChildStyle):
+
 2017-02-28  Antoine Quint  <graouts@apple.com>
 
         [Modern Media Controls] Add missing compact mode assets for macOS
index 484530f..22252d6 100644 (file)
@@ -69,7 +69,7 @@ void RenderButton::addChild(RenderObject* newChild, RenderObject* beforeChild)
         // Create an anonymous block.
         ASSERT(!firstChild());
         m_inner = createAnonymousBlock(style().display());
-        setupInnerStyle(&m_inner->mutableStyle());
+        updateAnonymousChildStyle(*m_inner, m_inner->mutableStyle());
         RenderFlexibleBox::addChild(m_inner);
     }
     
@@ -88,38 +88,23 @@ void RenderButton::removeChild(RenderObject& oldChild)
     } else
         m_inner->removeChild(oldChild);
 }
-
-void RenderButton::styleWillChange(StyleDifference diff, const RenderStyle& newStyle)
-{
-    if (m_inner) {
-        // RenderBlock::setStyle is going to apply a new style to the inner block, which
-        // will have the initial flex value, 0. The current value is 1, because we set
-        // it right below. Here we change it back to 0 to avoid getting a spurious layout hint
-        // because of the difference. Same goes for the other properties.
-        // FIXME: Make this hack unnecessary.
-        m_inner->mutableStyle().setFlexGrow(newStyle.initialFlexGrow());
-        m_inner->mutableStyle().setMarginTop(newStyle.initialMargin());
-        m_inner->mutableStyle().setMarginBottom(newStyle.initialMargin());
-    }
-    RenderBlock::styleWillChange(diff, newStyle);
-}
-
-void RenderButton::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
-{
-    RenderBlock::styleDidChange(diff, oldStyle);
-
-    if (m_inner) // RenderBlock handled updating the anonymous block's style.
-        setupInnerStyle(&m_inner->mutableStyle());
-}
-
-void RenderButton::setupInnerStyle(RenderStyle* innerStyle) 
+    
+void RenderButton::updateAnonymousChildStyle(const RenderObject& child, RenderStyle& childStyle) const
 {
-    innerStyle->setFlexGrow(1.0f);
+    ASSERT_UNUSED(child, !m_inner || &child == m_inner);
+    
+    childStyle.setFlexGrow(1.0f);
+    // min-width: 0; is needed for correct shrinking.
+    childStyle.setMinWidth(Length(0, Fixed));
     // Use margin:auto instead of align-items:center to get safe centering, i.e.
     // when the content overflows, treat it the same as align-items: flex-start.
-    innerStyle->setMarginTop(Length());
-    innerStyle->setMarginBottom(Length());
-    innerStyle->setFlexDirection(style().flexDirection());
+    childStyle.setMarginTop(Length());
+    childStyle.setMarginBottom(Length());
+    childStyle.setFlexDirection(style().flexDirection());
+    childStyle.setJustifyContent(style().justifyContent());
+    childStyle.setFlexWrap(style().flexWrap());
+    childStyle.setAlignItems(style().alignItems());
+    childStyle.setAlignContent(style().alignContent());
 }
 
 void RenderButton::updateFromElement()
index 96bfda8..2d96c6e 100644 (file)
@@ -46,13 +46,14 @@ public:
     void removeLeftoverAnonymousBlock(RenderBlock*) override { }
     bool createsAnonymousWrapper() const override { return true; }
 
-    void setupInnerStyle(RenderStyle*);
     void updateFromElement() override;
 
     bool canHaveGeneratedChildren() const override;
     bool hasControlClip() const override { return true; }
     LayoutRect controlClipRect(const LayoutPoint&) const override;
 
+    void updateAnonymousChildStyle(const RenderObject& anonymousChild, RenderStyle&) const override;
+
     void setText(const String&);
     String text() const;
 
@@ -66,9 +67,6 @@ private:
     const char* renderName() const override { return "RenderButton"; }
     bool isRenderButton() const override { return true; }
 
-    void styleWillChange(StyleDifference, const RenderStyle& newStyle) override;
-    void styleDidChange(StyleDifference, const RenderStyle* oldStyle) override;
-
     bool hasLineIfEmpty() const override;
 
     bool isFlexibleBoxImpl() const override { return true; }
index 56d68d2..d3944e5 100644 (file)
@@ -810,6 +810,8 @@ void RenderElement::propagateStyleToAnonymousChildren(StylePropagationType propa
         if (elementChild.isInFlowPositioned() && downcast<RenderBlock>(elementChild).isAnonymousBlockContinuation())
             newStyle.setPosition(elementChild.style().position());
 
+        updateAnonymousChildStyle(elementChild, newStyle);
+        
         elementChild.setStyle(WTFMove(newStyle));
     }
 }
index 36d0227..e19a38e 100644 (file)
@@ -216,6 +216,10 @@ public:
 
     void removeFromRenderFlowThread();
 
+    // Called before anonymousChild.setStyle(). Override to set custom styles for
+    // the child.
+    virtual void updateAnonymousChildStyle(const RenderObject&, RenderStyle&) const { };
+
 protected:
     enum BaseTypeFlag {
         RenderLayerModelObjectFlag  = 1 << 0,