Title: [156040] trunk
Revision
156040
Author
akl...@apple.com
Date
2013-09-18 08:35:01 -0700 (Wed, 18 Sep 2013)

Log Message

Avoid using RenderBR internally in RenderMenuList.
<https://webkit.org/b/121550>

Reviewed by Antti Koivisto.

Source/WebCore:

RenderMenuLists with an empty caption text were previously using a
RenderBR as placeholder.

Switch to using a \n RenderText instead so we can tighten RenderBR.

This will change DRT dumps but actual metrics should not change.

LayoutTests:

Update expected results for RenderMenuLists with empty text.
They now have a RenderText inside them instead of a RenderBR.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (156039 => 156040)


--- trunk/LayoutTests/ChangeLog	2013-09-18 14:41:45 UTC (rev 156039)
+++ trunk/LayoutTests/ChangeLog	2013-09-18 15:35:01 UTC (rev 156040)
@@ -1,3 +1,13 @@
+2013-09-18  Andreas Kling  <akl...@apple.com>
+
+        Avoid using RenderBR internally in RenderMenuList.
+        <https://webkit.org/b/121550>
+
+        Reviewed by Antti Koivisto.
+
+        Update expected results for RenderMenuLists with empty text.
+        They now have a RenderText inside them instead of a RenderBR.
+
 2013-09-18  Csaba Osztrogonác  <o...@webkit.org>
 
         ASSERT_NOT_REACHED is touched in WebCore::CSSPrimitiveValue::computeLengthDouble

Modified: trunk/LayoutTests/fast/flexbox/clear-overflow-before-scroll-update-expected.txt (156039 => 156040)


--- trunk/LayoutTests/fast/flexbox/clear-overflow-before-scroll-update-expected.txt	2013-09-18 14:41:45 UTC (rev 156039)
+++ trunk/LayoutTests/fast/flexbox/clear-overflow-before-scroll-update-expected.txt	2013-09-18 15:35:01 UTC (rev 156040)
@@ -11,4 +11,5 @@
 layer at (10,10) size 15x23 clip at (15,15) size 5x13
   RenderMenuList {SELECT} at (2,2) size 15x23 [bgcolor=#FFFFFF] [border: (5px solid #000000)]
     RenderBlock (anonymous) at (5,5) size 5x13
-      RenderBR at (-9999,0) size 0x13 [bgcolor=#FFFFFF]
+      RenderText at (-9999,0) size 0x13
+        text run at (-9999,0) width 0: " "

Modified: trunk/LayoutTests/platform/mac/fast/forms/HTMLOptionElement_label06-expected.txt (156039 => 156040)


--- trunk/LayoutTests/platform/mac/fast/forms/HTMLOptionElement_label06-expected.txt	2013-09-18 14:41:45 UTC (rev 156039)
+++ trunk/LayoutTests/platform/mac/fast/forms/HTMLOptionElement_label06-expected.txt	2013-09-18 15:35:01 UTC (rev 156040)
@@ -8,5 +8,6 @@
       RenderBR {BR} at (718,14) size 0x0
       RenderMenuList {SELECT} at (2,20) size 36x18 [bgcolor=#FFFFFF]
         RenderBlock (anonymous) at (0,0) size 36x18
-          RenderBR at (8,2) size 0x13 [bgcolor=#FFFFFF]
+          RenderText at (8,2) size 0x13
+            text run at (8,2) width 0: " "
       RenderText {#text} at (0,0) size 0x0

Modified: trunk/LayoutTests/platform/mac/fast/forms/HTMLOptionElement_label07-expected.txt (156039 => 156040)


--- trunk/LayoutTests/platform/mac/fast/forms/HTMLOptionElement_label07-expected.txt	2013-09-18 14:41:45 UTC (rev 156039)
+++ trunk/LayoutTests/platform/mac/fast/forms/HTMLOptionElement_label07-expected.txt	2013-09-18 15:35:01 UTC (rev 156040)
@@ -9,5 +9,6 @@
       RenderBR {BR} at (28,32) size 0x0
       RenderMenuList {SELECT} at (2,38) size 36x18 [bgcolor=#FFFFFF]
         RenderBlock (anonymous) at (0,0) size 36x18
-          RenderBR at (8,2) size 0x13 [bgcolor=#FFFFFF]
+          RenderText at (8,2) size 0x13
+            text run at (8,2) width 0: " "
       RenderText {#text} at (0,0) size 0x0

Modified: trunk/LayoutTests/platform/mac/fast/forms/form-element-geometry-expected.txt (156039 => 156040)


--- trunk/LayoutTests/platform/mac/fast/forms/form-element-geometry-expected.txt	2013-09-18 14:41:45 UTC (rev 156039)
+++ trunk/LayoutTests/platform/mac/fast/forms/form-element-geometry-expected.txt	2013-09-18 15:35:01 UTC (rev 156040)
@@ -188,7 +188,8 @@
           RenderText {#text} at (0,0) size 0x0
           RenderMenuList {SELECT} at (2,9) size 36x18 [bgcolor=#FFFFFF]
             RenderBlock (anonymous) at (0,0) size 36x18
-              RenderBR at (8,2) size 0x13 [bgcolor=#FFFFFF]
+              RenderText at (8,2) size 0x13
+                text run at (8,2) width 0: " "
           RenderText {#text} at (40,0) size 6x28
             text run at (40,0) width 6: " "
           RenderMenuList {SELECT} at (48,9) size 36x18 [bgcolor=#FFFFFF]
@@ -205,7 +206,8 @@
       RenderBlock {DIV} at (0,570) size 769x22
         RenderMenuList {SELECT} at (2,2) size 36x18 [bgcolor=#FFFFFF]
           RenderBlock (anonymous) at (0,0) size 36x18
-            RenderBR at (8,2) size 0x13 [bgcolor=#FFFFFF]
+            RenderText at (8,2) size 0x13
+              text run at (8,2) width 0: " "
         RenderText {#text} at (40,1) size 4x18
           text run at (40,1) width 4: " "
         RenderMenuList {SELECT} at (46,2) size 36x18 [bgcolor=#FFFFFF]
@@ -224,7 +226,8 @@
           RenderText {#text} at (0,0) size 0x0
           RenderMenuList {SELECT} at (2,2) size 36x18 [bgcolor=#FFFFFF]
             RenderBlock (anonymous) at (0,0) size 36x18
-              RenderBR at (8,2) size 0x13 [bgcolor=#FFFFFF]
+              RenderText at (8,2) size 0x13
+                text run at (8,2) width 0: " "
           RenderText {#text} at (40,5) size 3x13
             text run at (40,5) width 3: " "
           RenderMenuList {SELECT} at (45,2) size 36x18 [bgcolor=#FFFFFF]

Modified: trunk/LayoutTests/platform/mac/fast/forms/menulist-separator-painting-expected.txt (156039 => 156040)


--- trunk/LayoutTests/platform/mac/fast/forms/menulist-separator-painting-expected.txt	2013-09-18 14:41:45 UTC (rev 156039)
+++ trunk/LayoutTests/platform/mac/fast/forms/menulist-separator-painting-expected.txt	2013-09-18 15:35:01 UTC (rev 156040)
@@ -7,5 +7,6 @@
       RenderBlock (anonymous) at (0,6) size 784x22
         RenderMenuList {SELECT} at (2,2) size 36x18 [bgcolor=#FFFFFF] [border: (1px solid #008000)]
           RenderBlock (anonymous) at (1,1) size 34x16
-            RenderBR at (8,1) size 0x13 [bgcolor=#FFFFFF]
+            RenderText at (8,1) size 0x13
+              text run at (8,1) width 0: " "
         RenderText {#text} at (0,0) size 0x0

Modified: trunk/LayoutTests/platform/mac/fast/forms/select-baseline-expected.txt (156039 => 156040)


--- trunk/LayoutTests/platform/mac/fast/forms/select-baseline-expected.txt	2013-09-18 14:41:45 UTC (rev 156039)
+++ trunk/LayoutTests/platform/mac/fast/forms/select-baseline-expected.txt	2013-09-18 15:35:01 UTC (rev 156040)
@@ -8,7 +8,8 @@
       RenderBR {BR} at (462,14) size 0x0
       RenderMenuList {SELECT} at (2,22) size 36x18 [bgcolor=#FFFFFF]
         RenderBlock (anonymous) at (0,0) size 36x18
-          RenderBR at (8,2) size 0x13 [bgcolor=#FFFFFF]
+          RenderText at (8,2) size 0x13
+            text run at (8,2) width 0: " "
       RenderText {#text} at (40,21) size 29x18
         text run at (40,21) width 29: " test "
       RenderMenuList {SELECT} at (71,22) size 51x18 [bgcolor=#FFFFFF]
@@ -19,7 +20,8 @@
         text run at (124,21) width 4: " "
       RenderMenuList {SELECT} at (130,22) size 36x18 [color=#00008B] [bgcolor=#ADD8E6] [border: (1px solid #00008B)]
         RenderBlock (anonymous) at (1,1) size 34x16
-          RenderBR at (8,1) size 0x13 [bgcolor=#ADD8E6]
+          RenderText at (8,1) size 0x13
+            text run at (8,1) width 0: " "
       RenderText {#text} at (168,21) size 29x18
         text run at (168,21) width 29: " test "
       RenderMenuList {SELECT} at (199,22) size 51x18 [color=#00008B] [bgcolor=#ADD8E6] [border: (1px solid #00008B)]

Modified: trunk/LayoutTests/platform/mac/fast/forms/selectlist-minsize-expected.txt (156039 => 156040)


--- trunk/LayoutTests/platform/mac/fast/forms/selectlist-minsize-expected.txt	2013-09-18 14:41:45 UTC (rev 156039)
+++ trunk/LayoutTests/platform/mac/fast/forms/selectlist-minsize-expected.txt	2013-09-18 15:35:01 UTC (rev 156040)
@@ -5,5 +5,6 @@
     RenderBody {BODY} at (8,8) size 784x22
       RenderMenuList {SELECT} at (2,2) size 36x18 [bgcolor=#FFFFFF]
         RenderBlock (anonymous) at (0,0) size 36x18
-          RenderBR at (8,2) size 0x13 [bgcolor=#FFFFFF]
+          RenderText at (8,2) size 0x13
+            text run at (8,2) width 0: " "
       RenderText {#text} at (0,0) size 0x0

Modified: trunk/LayoutTests/platform/mac/fast/replaced/three-selects-break-expected.txt (156039 => 156040)


--- trunk/LayoutTests/platform/mac/fast/replaced/three-selects-break-expected.txt	2013-09-18 14:41:45 UTC (rev 156039)
+++ trunk/LayoutTests/platform/mac/fast/replaced/three-selects-break-expected.txt	2013-09-18 15:35:01 UTC (rev 156040)
@@ -6,10 +6,13 @@
       RenderBlock {DIV} at (0,0) size 5x66
         RenderMenuList {SELECT} at (2,2) size 36x18 [bgcolor=#FFFFFF]
           RenderBlock (anonymous) at (0,0) size 36x18
-            RenderBR at (8,2) size 0x13 [bgcolor=#FFFFFF]
+            RenderText at (8,2) size 0x13
+              text run at (8,2) width 0: " "
         RenderMenuList {SELECT} at (2,24) size 36x18 [bgcolor=#FFFFFF]
           RenderBlock (anonymous) at (0,0) size 36x18
-            RenderBR at (8,2) size 0x13 [bgcolor=#FFFFFF]
+            RenderText at (8,2) size 0x13
+              text run at (8,2) width 0: " "
         RenderMenuList {SELECT} at (2,46) size 36x18 [bgcolor=#FFFFFF]
           RenderBlock (anonymous) at (0,0) size 36x18
-            RenderBR at (8,2) size 0x13 [bgcolor=#FFFFFF]
+            RenderText at (8,2) size 0x13
+              text run at (8,2) width 0: " "

Modified: trunk/Source/WebCore/ChangeLog (156039 => 156040)


--- trunk/Source/WebCore/ChangeLog	2013-09-18 14:41:45 UTC (rev 156039)
+++ trunk/Source/WebCore/ChangeLog	2013-09-18 15:35:01 UTC (rev 156040)
@@ -1,3 +1,17 @@
+2013-09-18  Andreas Kling  <akl...@apple.com>
+
+        Avoid using RenderBR internally in RenderMenuList.
+        <https://webkit.org/b/121550>
+
+        Reviewed by Antti Koivisto.
+
+        RenderMenuLists with an empty caption text were previously using a
+        RenderBR as placeholder.
+
+        Switch to using a \n RenderText instead so we can tighten RenderBR.
+
+        This will change DRT dumps but actual metrics should not change.
+
 2013-09-18  Antti Koivisto  <an...@apple.com>
 
         Change one accidental "object->isText() || object->isLineBreak()" from previous patch back to "object->isTextOrLineBreak()"

Modified: trunk/Source/WebCore/rendering/RenderMenuList.cpp (156039 => 156040)


--- trunk/Source/WebCore/rendering/RenderMenuList.cpp	2013-09-18 14:41:45 UTC (rev 156039)
+++ trunk/Source/WebCore/rendering/RenderMenuList.cpp	2013-09-18 15:35:01 UTC (rev 156040)
@@ -39,7 +39,6 @@
 #include "NodeRenderStyle.h"
 #include "Page.h"
 #include "PopupMenu.h"
-#include "RenderBR.h"
 #include "RenderScrollbar.h"
 #include "RenderText.h"
 #include "RenderTheme.h"
@@ -58,7 +57,6 @@
 RenderMenuList::RenderMenuList(HTMLSelectElement& element)
     : RenderFlexibleBox(&element)
     , m_buttonText(nullptr)
-    , m_buttonBR(nullptr)
     , m_innerBlock(nullptr)
     , m_needsOptionsWidthUpdate(true)
     , m_optionsWidth(0)
@@ -162,8 +160,6 @@
 
     if (m_buttonText)
         m_buttonText->setStyle(style());
-    if (m_buttonBR)
-        m_buttonBR->setStyle(style());
     if (m_innerBlock) // RenderBlock handled updating the anonymous block's style.
         adjustInnerStyle();
 
@@ -243,36 +239,21 @@
 
 void RenderMenuList::setText(const String& s)
 {
-    if (s.isEmpty()) {
-        if (!m_buttonBR) {
-            if (m_buttonText) {
-                m_buttonText->destroy();
-                m_buttonText = nullptr;
-            }
-            // FIXME: This could probably just be a text node.
-            m_buttonBR = RenderBR::createAnonymous(document());
-            m_buttonBR->setStyle(style());
-            addChild(m_buttonBR);
-        }
-    } else {
-        if (m_buttonText)
-            m_buttonText->setText(s.impl(), true);
-        else {
-            if (m_buttonBR) {
-                m_buttonBR->destroy();
-                m_buttonBR = nullptr;
-            }
-            m_buttonText = new (renderArena()) RenderText(&document(), s.impl());
-            m_buttonText->setStyle(style());
-            addChild(m_buttonText);
-        }
-        adjustInnerStyle();
+    String textToUse = s.isEmpty() ? String(ASCIILiteral("\n")) : s;
+
+    if (m_buttonText)
+        m_buttonText->setText(textToUse.impl(), true);
+    else {
+        m_buttonText = new (renderArena()) RenderText(&document(), textToUse.impl());
+        m_buttonText->setStyle(style());
+        addChild(m_buttonText);
     }
+    adjustInnerStyle();
 }
 
 String RenderMenuList::text() const
 {
-    return m_buttonText ? m_buttonText->text() : m_buttonBR ? String(ASCIILiteral("\n")) : String();
+    return m_buttonText ? m_buttonText->text() : String();
 }
 
 LayoutRect RenderMenuList::controlClipRect(const LayoutPoint& additionalOffset) const

Modified: trunk/Source/WebCore/rendering/RenderMenuList.h (156039 => 156040)


--- trunk/Source/WebCore/rendering/RenderMenuList.h	2013-09-18 14:41:45 UTC (rev 156039)
+++ trunk/Source/WebCore/rendering/RenderMenuList.h	2013-09-18 15:35:01 UTC (rev 156040)
@@ -38,7 +38,6 @@
 namespace WebCore {
 
 class HTMLSelectElement;
-class RenderBR;
 class RenderText;
 
 class RenderMenuList FINAL : public RenderFlexibleBox, private PopupMenuClient {
@@ -137,7 +136,6 @@
     void didUpdateActiveOption(int optionIndex);
 
     RenderText* m_buttonText;
-    RenderBR* m_buttonBR;
     RenderBlock* m_innerBlock;
 
     bool m_needsOptionsWidthUpdate;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to