Remove HTMLTextFormControl::fixPlaceholderRenderer
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Sep 2013 01:08:47 +0000 (01:08 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Sep 2013 01:08:47 +0000 (01:08 +0000)
https://bugs.webkit.org/show_bug.cgi?id=121058

Reviewed by Kent Tamura.

Source/WebCore:

HTMLTextFormControl::fixPlaceholderRenderer was added in r118733 to swap the order in which placeholder appears.
Namely, when a text form control element is focused, placeholder should be rendered behind the text for the caret
to render on top of, not behind, the placeholder text.  However, we can achieve the same effect by changing
the order of elements in the shadow DOM instead of manually manipuating the render tree as the assertion failure
mentioned in the change log is a bogus one.

Cleaned up the code by removing HTMLTextFormControl::fixPlaceholderRenderer and changed the order in which inner
text element and placeholder element appear. Also fixed the assertion in Position's constructor.

* dom/Position.cpp:
(WebCore::Position::Position): The original assertion was bogus. What we don't want have is for a Position to be
before or after a shadow root. It's fine for it be anchroed against the shadow root as long as it's inside.

* html/HTMLTextAreaElement.cpp:
(WebCore::HTMLTextAreaElement::updatePlaceholderText): Insert the placeholder before the inner text element.
* html/HTMLTextAreaElement.h:
* html/HTMLTextFormControlElement.cpp:
* html/HTMLTextFormControlElement.h:
* html/TextFieldInputType.cpp:
(WebCore::TextFieldInputType::updatePlaceholderText): Ditto.
* html/TextFieldInputType.h:

LayoutTests:

Rebaseline tests now that the placeholder appears before the inner text element.

* fast/forms/placeholder-position-expected.txt:
* platform/mac/fast/forms/input-placeholder-visibility-1-expected.txt:
* platform/mac/fast/forms/input-placeholder-visibility-3-expected.txt:
* platform/mac/fast/forms/textarea-placeholder-pseudo-style-expected.txt:
* platform/mac/fast/forms/textarea-placeholder-visibility-1-expected.txt:
* platform/mac/fast/forms/textarea-placeholder-visibility-2-expected.txt:

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

15 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/forms/placeholder-position-expected.txt
LayoutTests/platform/mac/fast/forms/input-placeholder-visibility-1-expected.txt
LayoutTests/platform/mac/fast/forms/input-placeholder-visibility-3-expected.txt
LayoutTests/platform/mac/fast/forms/textarea-placeholder-pseudo-style-expected.txt
LayoutTests/platform/mac/fast/forms/textarea-placeholder-visibility-1-expected.txt
LayoutTests/platform/mac/fast/forms/textarea-placeholder-visibility-2-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/dom/Position.cpp
Source/WebCore/html/HTMLTextAreaElement.cpp
Source/WebCore/html/HTMLTextAreaElement.h
Source/WebCore/html/HTMLTextFormControlElement.cpp
Source/WebCore/html/HTMLTextFormControlElement.h
Source/WebCore/html/TextFieldInputType.cpp
Source/WebCore/html/TextFieldInputType.h

index a482b2a8470c6a93baadf1cc349325cfc734881d..29e4a53762889d602448439b561b4056f4df5f06 100644 (file)
@@ -1,3 +1,19 @@
+2013-09-09  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Remove HTMLTextFormControl::fixPlaceholderRenderer
+        https://bugs.webkit.org/show_bug.cgi?id=121058
+
+        Reviewed by Kent Tamura.
+
+        Rebaseline tests now that the placeholder appears before the inner text element.
+
+        * fast/forms/placeholder-position-expected.txt:
+        * platform/mac/fast/forms/input-placeholder-visibility-1-expected.txt:
+        * platform/mac/fast/forms/input-placeholder-visibility-3-expected.txt:
+        * platform/mac/fast/forms/textarea-placeholder-pseudo-style-expected.txt:
+        * platform/mac/fast/forms/textarea-placeholder-visibility-1-expected.txt:
+        * platform/mac/fast/forms/textarea-placeholder-visibility-2-expected.txt:
+
 2013-09-09  Mark Lam  <mark.lam@apple.com>
 
         Rolling out r155392(Remove old fast/js/resources pre and post test files): breaks tests.
index d9caf1377240a8de06999343edab538f6e7d39f8..8c4096f98a4f4b5f4371b01f44855aeece3c9ae2 100644 (file)
@@ -73,10 +73,10 @@ layer at (35,82) size 117x13
   RenderBlock {DIV} at (0,0) size 117x13
 layer at (10,102) size 161x32 clip at (11,103) size 159x30
   RenderTextControl {TEXTAREA} at (2,94) size 161x32 [bgcolor=#FFFFFF] [border: (1px solid #000000)]
+    RenderBlock {DIV} at (3,3) size 155x13
     RenderBlock {DIV} at (3,3) size 155x13 [color=#A9A9A9]
       RenderText {#text} at (0,0) size 63x13
         text run at (0,0) width 63: "placeholder"
-    RenderBlock {DIV} at (3,3) size 155x13
 layer at (13,141) size 117x13
   RenderBlock {DIV} at (3,3) size 117x13 [color=#A9A9A9]
     RenderText {#text} at (0,0) size 63x13
@@ -85,10 +85,10 @@ layer at (13,141) size 117x13
   RenderBlock {DIV} at (3,3) size 117x13
 layer at (10,175) size 161x45 clip at (11,176) size 159x43
   RenderTextControl {TEXTAREA} at (2,167) size 161x45 [bgcolor=#FFFFFF] [border: (1px solid #000000)]
+    RenderBlock {DIV} at (3,16) size 155x13
     RenderBlock {DIV} at (3,16) size 155x13 [color=#A9A9A9]
       RenderText {#text} at (0,0) size 63x13
         text run at (0,0) width 63: "placeholder"
-    RenderBlock {DIV} at (3,16) size 155x13
 layer at (19,233) size 162x18
   RenderBlock {DIV} at (6,6) size 162x18 [color=#A9A9A9]
     RenderText {#text} at (0,0) size 85x18
index 9a8f3a7f9acc50241b7162902de65366a30eceb9..eed30c803f11c974efb9694df99ebc8359293665 100644 (file)
@@ -16,4 +16,4 @@ layer at (13,47) size 117x13
       text run at (0,0) width 62: "Placeholder"
 layer at (13,47) size 117x13
   RenderBlock {DIV} at (3,3) size 117x13
-caret: position 0 of child 0 {DIV} of {#document-fragment} of child 1 {INPUT} of child 3 {DIV} of body
+caret: position 0 of child 1 {DIV} of {#document-fragment} of child 1 {INPUT} of child 3 {DIV} of body
index a02d029da0805d4be24c13e19bc0bc068f99c5c5..1b4c3b27d16b0b7501d801a7ad74ad68a8439672 100644 (file)
@@ -17,4 +17,4 @@ layer at (13,47) size 117x13
 layer at (13,47) size 117x13
   RenderBlock {DIV} at (3,3) size 117x13
     RenderBR {BR} at (0,0) size 0x13
-caret: position 0 of child 0 {BR} of child 0 {DIV} of {#document-fragment} of child 1 {INPUT} of child 3 {DIV} of body
+caret: position 0 of child 0 {BR} of child 1 {DIV} of {#document-fragment} of child 1 {INPUT} of child 3 {DIV} of body
index bad585118f6ca2261642be261c1ea84f613ad83b..e91fd90893244c6938229742e7d55faa56ee5fc1 100644 (file)
@@ -15,25 +15,25 @@ layer at (0,0) size 800x600
       RenderText {#text} at (0,0) size 0x0
 layer at (10,28) size 161x32 clip at (11,29) size 159x30
   RenderTextControl {TEXTAREA} at (2,20) size 161x32 [bgcolor=#FFFFFF] [border: (1px solid #000000)]
+    RenderBlock {DIV} at (3,3) size 155x13
     RenderBlock {DIV} at (3,3) size 155x13 [color=#640000]
       RenderText {#text} at (0,0) size 22x13
         text run at (0,0) width 22: "text"
-    RenderBlock {DIV} at (3,3) size 155x13
 layer at (179,28) size 161x32 clip at (180,29) size 159x30
   RenderTextControl {TEXTAREA} at (171,20) size 161x32 [bgcolor=#FFFFFF] [border: (1px solid #000000)]
+    RenderBlock {DIV} at (3,3) size 155x13 [color=#545454]
     RenderBlock {DIV} at (3,3) size 155x13 [color=#640000]
       RenderText {#text} at (0,0) size 70x13
         text run at (0,0) width 70: "disabled text"
-    RenderBlock {DIV} at (3,3) size 155x13 [color=#545454]
 layer at (348,28) size 161x32 clip at (349,29) size 159x30
   RenderTextControl {TEXTAREA} at (340,20) size 161x32 [bgcolor=#FFFFFF] [border: (1px solid #000000)]
+    RenderBlock {DIV} at (3,3) size 155x13
     RenderBlock {DIV} at (3,3) size 155x13 [color=#A9A9A9]
       RenderText {#text} at (0,0) size 38x13
         text run at (0,0) width 38: "default"
-    RenderBlock {DIV} at (3,3) size 155x13
 layer at (517,28) size 161x32 clip at (518,29) size 159x30
   RenderTextControl {TEXTAREA} at (509,20) size 161x32 [bgcolor=#FFFFFF] [border: (1px solid #000000)]
+    RenderBlock {DIV} at (3,3) size 155x13 [color=#545454]
     RenderBlock {DIV} at (3,3) size 155x13 [color=#A9A9A9]
       RenderText {#text} at (0,0) size 86x13
         text run at (0,0) width 86: "default disabled"
-    RenderBlock {DIV} at (3,3) size 155x13 [color=#545454]
index c3b730fd3f00e765c76790858c6697f7a0d84bf3..b865c4408163c10008d891b210d8844906d89abd 100644 (file)
@@ -11,9 +11,9 @@ layer at (0,0) size 800x600
         RenderText {#text} at (0,0) size 0x0
 layer at (10,44) size 161x32 clip at (11,45) size 159x30
   RenderTextControl {TEXTAREA} at (2,2) size 161x32 [bgcolor=#FFFFFF] [border: (1px solid #000000)]
+    RenderBlock {DIV} at (3,3) size 155x13
+      RenderBR {BR} at (0,0) size 0x13
     RenderBlock {DIV} at (3,3) size 155x13 [color=#A9A9A9]
       RenderText {#text} at (0,0) size 62x13
         text run at (0,0) width 62: "Placeholder"
-    RenderBlock {DIV} at (3,3) size 155x13
-      RenderBR {BR} at (0,0) size 0x13
 caret: position 0 of child 0 {BR} of child 0 {DIV} of {#document-fragment} of child 1 {TEXTAREA} of child 3 {DIV} of body
index ae02d1296a201ac77e9423a11333d5a06e625f05..655c41ae5a9600c9babc8cc0c53d0d1630881a2a 100644 (file)
@@ -11,8 +11,8 @@ layer at (0,0) size 800x600
         RenderText {#text} at (0,0) size 0x0
 layer at (10,44) size 161x32 clip at (11,45) size 159x30
   RenderTextControl {TEXTAREA} at (2,2) size 161x32 [bgcolor=#FFFFFF] [border: (1px solid #000000)]
+    RenderBlock {DIV} at (3,3) size 155x13
     RenderBlock {DIV} at (3,3) size 155x13 [color=#A9A9A9]
       RenderText {#text} at (0,0) size 62x13
         text run at (0,0) width 62: "Placeholder"
-    RenderBlock {DIV} at (3,3) size 155x13
 caret: position 0 of child 0 {DIV} of {#document-fragment} of child 1 {TEXTAREA} of child 3 {DIV} of body
index 87adc451e39e21e3d736cf43a9186ee1349919db..5c16fc24ae67704a8ecde38c80ac283299c2af0b 100644 (file)
@@ -1,3 +1,32 @@
+2013-09-09  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Remove HTMLTextFormControl::fixPlaceholderRenderer
+        https://bugs.webkit.org/show_bug.cgi?id=121058
+
+        Reviewed by Kent Tamura.
+
+        HTMLTextFormControl::fixPlaceholderRenderer was added in r118733 to swap the order in which placeholder appears.
+        Namely, when a text form control element is focused, placeholder should be rendered behind the text for the caret
+        to render on top of, not behind, the placeholder text.  However, we can achieve the same effect by changing
+        the order of elements in the shadow DOM instead of manually manipuating the render tree as the assertion failure
+        mentioned in the change log is a bogus one.
+
+        Cleaned up the code by removing HTMLTextFormControl::fixPlaceholderRenderer and changed the order in which inner
+        text element and placeholder element appear. Also fixed the assertion in Position's constructor.
+
+        * dom/Position.cpp:
+        (WebCore::Position::Position): The original assertion was bogus. What we don't want have is for a Position to be
+        before or after a shadow root. It's fine for it be anchroed against the shadow root as long as it's inside.
+
+        * html/HTMLTextAreaElement.cpp:
+        (WebCore::HTMLTextAreaElement::updatePlaceholderText): Insert the placeholder before the inner text element.
+        * html/HTMLTextAreaElement.h:
+        * html/HTMLTextFormControlElement.cpp:
+        * html/HTMLTextFormControlElement.h:
+        * html/TextFieldInputType.cpp:
+        (WebCore::TextFieldInputType::updatePlaceholderText): Ditto.
+        * html/TextFieldInputType.h:
+
 2013-09-09  Andreas Kling  <akling@apple.com>
 
         Hide node() below RenderSVGModelObject, use element() instead.
index 73d82808e4585283bac1b17cb92d79a8301469fe..bb62ffdcfd84ae6dc0530d102a811bcca96146ea 100644 (file)
@@ -85,10 +85,9 @@ Position::Position(PassRefPtr<Node> anchorNode, LegacyEditingOffset offset)
     , m_isLegacyEditingPosition(true)
 {
 #if ENABLE(SHADOW_DOM)
-    ASSERT((m_anchorNode && RuntimeEnabledFeatures::shadowDOMEnabled())
-           || !m_anchorNode || !m_anchorNode->isShadowRoot());
+    ASSERT((m_anchorNode && RuntimeEnabledFeatures::shadowDOMEnabled()) || !m_anchorNode || !m_anchorNode->isShadowRoot() || m_anchorNode == containerNode());
 #else
-    ASSERT(!m_anchorNode || !m_anchorNode->isShadowRoot());
+    ASSERT(!m_anchorNode || !m_anchorNode->isShadowRoot() || m_anchorNode == containerNode());
 #endif
     ASSERT(!m_anchorNode || !m_anchorNode->isPseudoElement());
 }
@@ -100,10 +99,9 @@ Position::Position(PassRefPtr<Node> anchorNode, AnchorType anchorType)
     , m_isLegacyEditingPosition(false)
 {
 #if ENABLE(SHADOW_DOM)
-    ASSERT((m_anchorNode && RuntimeEnabledFeatures::shadowDOMEnabled())
-           || !m_anchorNode || !m_anchorNode->isShadowRoot());
+    ASSERT((m_anchorNode && RuntimeEnabledFeatures::shadowDOMEnabled()) || !m_anchorNode || !m_anchorNode->isShadowRoot() || m_anchorNode == containerNode());
 #else
-    ASSERT(!m_anchorNode || !m_anchorNode->isShadowRoot());
+    ASSERT(!m_anchorNode || !m_anchorNode->isShadowRoot() || m_anchorNode == containerNode());
 #endif
 
     ASSERT(!m_anchorNode || !m_anchorNode->isPseudoElement());
index a0493043b69de7f7667700d1838b7219ad9787a8..0fc3f330f9f2265eaa0f8a486d0b6b5c8022522a 100644 (file)
@@ -517,12 +517,6 @@ HTMLElement* HTMLTextAreaElement::placeholderElement() const
     return m_placeholder;
 }
 
-void HTMLTextAreaElement::didAttachRenderers()
-{
-    HTMLTextFormControlElement::didAttachRenderers();
-    fixPlaceholderRenderer(m_placeholder, innerTextElement());
-}
-
 bool HTMLTextAreaElement::matchesReadOnlyPseudoClass() const
 {
     return isReadOnly();
@@ -547,10 +541,9 @@ void HTMLTextAreaElement::updatePlaceholderText()
         RefPtr<HTMLDivElement> placeholder = HTMLDivElement::create(&document());
         m_placeholder = placeholder.get();
         m_placeholder->setPseudo(AtomicString("-webkit-input-placeholder", AtomicString::ConstructFromLiteral));
-        userAgentShadowRoot()->insertBefore(m_placeholder, innerTextElement()->nextSibling(), ASSERT_NO_EXCEPTION);
+        userAgentShadowRoot()->insertBefore(m_placeholder, innerTextElement()->nextSibling());
     }
     m_placeholder->setInnerText(placeholderText, ASSERT_NO_EXCEPTION);
-    fixPlaceholderRenderer(m_placeholder, innerTextElement());
 }
 
 }
index 38d96d5a88526a1e6f0a5ea3b1440d831a3dafa5..05c9596c0bd2e027a90a1eb6d8fa0a8cfb224130 100644 (file)
@@ -111,7 +111,6 @@ private:
     virtual void accessKeyAction(bool sendMouseEvents);
 
     virtual bool shouldUseInputMethod() OVERRIDE;
-    virtual void didAttachRenderers() OVERRIDE;
     virtual bool matchesReadOnlyPseudoClass() const OVERRIDE;
     virtual bool matchesReadWritePseudoClass() const OVERRIDE;
 
index 3d07a02612c7779949f92f9559d51093f0db9dee..541b8d8f2dec7410be0bcf9caa56dc8bedfc0e4f 100644 (file)
@@ -168,24 +168,6 @@ void HTMLTextFormControlElement::updatePlaceholderVisibility(bool placeholderVal
     placeholder->setInlineStyleProperty(CSSPropertyVisibility, placeholderShouldBeVisible() ? CSSValueVisible : CSSValueHidden);
 }
 
-void HTMLTextFormControlElement::fixPlaceholderRenderer(HTMLElement* placeholder, HTMLElement* siblingElement)
-{
-    // FIXME: We should change the order of DOM nodes. But it makes an assertion
-    // failure in editing code.
-    if (!placeholder || !placeholder->renderer())
-        return;
-    RenderObject* placeholderRenderer = placeholder->renderer();
-    RenderObject* siblingRenderer = siblingElement->renderer();
-    if (!siblingRenderer)
-        return;
-    if (placeholderRenderer->nextSibling() == siblingRenderer)
-        return;
-    RenderObject* parentRenderer = placeholderRenderer->parent();
-    ASSERT(siblingRenderer->parent() == parentRenderer);
-    parentRenderer->removeChild(placeholderRenderer);
-    parentRenderer->addChild(placeholderRenderer, siblingRenderer);
-}
-
 void HTMLTextFormControlElement::setSelectionStart(int start)
 {
     setSelectionRange(start, max(start, selectionEnd()), selectionDirection());
index 3a4fe97d886bfc24cf0681fa9a54d4b9a64fbede..ffd4d1981010ba92767e15f9e69095ad8a77f8f9 100644 (file)
@@ -54,7 +54,6 @@ public:
     bool placeholderShouldBeVisible() const;
     virtual HTMLElement* placeholderElement() const = 0;
     void updatePlaceholderVisibility(bool);
-    static void fixPlaceholderRenderer(HTMLElement* placeholder, HTMLElement* siblingElement);
 
     int indexForVisiblePosition(const VisiblePosition&) const;
     VisiblePosition visiblePositionForIndex(int index) const;
index 4d94ad9372b3a8224889c472d9baef0d227ea621..8228eb4b4fe88638132aaa96edcd336fc97f4981 100644 (file)
@@ -418,19 +418,9 @@ void TextFieldInputType::updatePlaceholderText()
     if (!m_placeholder) {
         m_placeholder = HTMLDivElement::create(&element()->document());
         m_placeholder->setPseudo(AtomicString("-webkit-input-placeholder", AtomicString::ConstructFromLiteral));
-        element()->userAgentShadowRoot()->insertBefore(m_placeholder, m_container ? m_container->nextSibling() : innerTextElement()->nextSibling(), ASSERT_NO_EXCEPTION);
+        element()->userAgentShadowRoot()->insertBefore(m_placeholder, m_container ? m_container.get() : innerTextElement(), ASSERT_NO_EXCEPTION);
     }
     m_placeholder->setInnerText(placeholderText, ASSERT_NO_EXCEPTION);
-    element()->fixPlaceholderRenderer(m_placeholder.get(), m_container ? m_container.get() : m_innerText.get());
-}
-
-void TextFieldInputType::attach()
-{
-    InputType::attach();
-    // If container exists, the container should not have any content data.
-    ASSERT(!m_container || !m_container->renderStyle() || !m_container->renderStyle()->hasContent());
-
-    element()->fixPlaceholderRenderer(m_placeholder.get(), m_container ? m_container.get() : m_innerText.get());
 }
 
 bool TextFieldInputType::appendFormData(FormDataList& list, bool multipart) const
index 86541f9463ddc70b29d1d4b8231d254b44be52ea..f3e68e1776969ac93fbd7eba660d1ae923ec8e84 100644 (file)
@@ -57,7 +57,6 @@ protected:
 #endif
 
 protected:
-    virtual void attach() OVERRIDE;
     virtual bool needsContainer() const;
     virtual bool shouldHaveSpinButton() const;
     virtual void createShadowSubtree() OVERRIDE;