Changeset 155408 in webkit


Ignore:
Timestamp:
Sep 9, 2013 6:08:47 PM (11 years ago)
Author:
rniwa@webkit.org
Message:

Remove HTMLTextFormControl::fixPlaceholderRenderer
​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:
Location:
trunk
Files:
15 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r155404 r155408  
     12013-09-09  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Remove HTMLTextFormControl::fixPlaceholderRenderer
     4        https://bugs.webkit.org/show_bug.cgi?id=121058
     5
     6        Reviewed by Kent Tamura.
     7
     8        Rebaseline tests now that the placeholder appears before the inner text element.
     9
     10        * fast/forms/placeholder-position-expected.txt:
     11        * platform/mac/fast/forms/input-placeholder-visibility-1-expected.txt:
     12        * platform/mac/fast/forms/input-placeholder-visibility-3-expected.txt:
     13        * platform/mac/fast/forms/textarea-placeholder-pseudo-style-expected.txt:
     14        * platform/mac/fast/forms/textarea-placeholder-visibility-1-expected.txt:
     15        * platform/mac/fast/forms/textarea-placeholder-visibility-2-expected.txt:
     16
    1172013-09-09  Mark Lam  <mark.lam@apple.com>
    218

  • trunk/LayoutTests/fast/forms/placeholder-position-expected.txt

    r155324 r155408  
    7474layer at (10,102) size 161x32 clip at (11,103) size 159x30
    7575  RenderTextControl {TEXTAREA} at (2,94) size 161x32 [bgcolor=#FFFFFF] [border: (1px solid #000000)]
     76    RenderBlock {DIV} at (3,3) size 155x13
    7677    RenderBlock {DIV} at (3,3) size 155x13 [color=#A9A9A9]
    7778      RenderText {#text} at (0,0) size 63x13
    7879        text run at (0,0) width 63: "placeholder"
    79     RenderBlock {DIV} at (3,3) size 155x13
    8080layer at (13,141) size 117x13
    8181  RenderBlock {DIV} at (3,3) size 117x13 [color=#A9A9A9]
    … …  
    8686layer at (10,175) size 161x45 clip at (11,176) size 159x43
    8787  RenderTextControl {TEXTAREA} at (2,167) size 161x45 [bgcolor=#FFFFFF] [border: (1px solid #000000)]
     88    RenderBlock {DIV} at (3,16) size 155x13
    8889    RenderBlock {DIV} at (3,16) size 155x13 [color=#A9A9A9]
    8990      RenderText {#text} at (0,0) size 63x13
    9091        text run at (0,0) width 63: "placeholder"
    91     RenderBlock {DIV} at (3,16) size 155x13
    9292layer at (19,233) size 162x18
    9393  RenderBlock {DIV} at (6,6) size 162x18 [color=#A9A9A9]

  • trunk/LayoutTests/platform/mac/fast/forms/input-placeholder-visibility-1-expected.txt

    r139198 r155408  
    1717layer at (13,47) size 117x13
    1818  RenderBlock {DIV} at (3,3) size 117x13
    19 caret: position 0 of child 0 {DIV} of {#document-fragment} of child 1 {INPUT} of child 3 {DIV} of body
     19caret: position 0 of child 1 {DIV} of {#document-fragment} of child 1 {INPUT} of child 3 {DIV} of body

  • trunk/LayoutTests/platform/mac/fast/forms/input-placeholder-visibility-3-expected.txt

    r139198 r155408  
    1818  RenderBlock {DIV} at (3,3) size 117x13
    1919    RenderBR {BR} at (0,0) size 0x13
    20 caret: position 0 of child 0 {BR} of child 0 {DIV} of {#document-fragment} of child 1 {INPUT} of child 3 {DIV} of body
     20caret: position 0 of child 0 {BR} of child 1 {DIV} of {#document-fragment} of child 1 {INPUT} of child 3 {DIV} of body

  • trunk/LayoutTests/platform/mac/fast/forms/textarea-placeholder-pseudo-style-expected.txt

    r118751 r155408  
    1616layer at (10,28) size 161x32 clip at (11,29) size 159x30
    1717  RenderTextControl {TEXTAREA} at (2,20) size 161x32 [bgcolor=#FFFFFF] [border: (1px solid #000000)]
     18    RenderBlock {DIV} at (3,3) size 155x13
    1819    RenderBlock {DIV} at (3,3) size 155x13 [color=#640000]
    1920      RenderText {#text} at (0,0) size 22x13
    2021        text run at (0,0) width 22: "text"
    21     RenderBlock {DIV} at (3,3) size 155x13
    2222layer at (179,28) size 161x32 clip at (180,29) size 159x30
    2323  RenderTextControl {TEXTAREA} at (171,20) size 161x32 [bgcolor=#FFFFFF] [border: (1px solid #000000)]
     24    RenderBlock {DIV} at (3,3) size 155x13 [color=#545454]
    2425    RenderBlock {DIV} at (3,3) size 155x13 [color=#640000]
    2526      RenderText {#text} at (0,0) size 70x13
    2627        text run at (0,0) width 70: "disabled text"
    27     RenderBlock {DIV} at (3,3) size 155x13 [color=#545454]
    2828layer at (348,28) size 161x32 clip at (349,29) size 159x30
    2929  RenderTextControl {TEXTAREA} at (340,20) size 161x32 [bgcolor=#FFFFFF] [border: (1px solid #000000)]
     30    RenderBlock {DIV} at (3,3) size 155x13
    3031    RenderBlock {DIV} at (3,3) size 155x13 [color=#A9A9A9]
    3132      RenderText {#text} at (0,0) size 38x13
    3233        text run at (0,0) width 38: "default"
    33     RenderBlock {DIV} at (3,3) size 155x13
    3434layer at (517,28) size 161x32 clip at (518,29) size 159x30
    3535  RenderTextControl {TEXTAREA} at (509,20) size 161x32 [bgcolor=#FFFFFF] [border: (1px solid #000000)]
     36    RenderBlock {DIV} at (3,3) size 155x13 [color=#545454]
    3637    RenderBlock {DIV} at (3,3) size 155x13 [color=#A9A9A9]
    3738      RenderText {#text} at (0,0) size 86x13
    3839        text run at (0,0) width 86: "default disabled"
    39     RenderBlock {DIV} at (3,3) size 155x13 [color=#545454]

  • trunk/LayoutTests/platform/mac/fast/forms/textarea-placeholder-visibility-1-expected.txt

    r139198 r155408  
    1212layer at (10,44) size 161x32 clip at (11,45) size 159x30
    1313  RenderTextControl {TEXTAREA} at (2,2) size 161x32 [bgcolor=#FFFFFF] [border: (1px solid #000000)]
     14    RenderBlock {DIV} at (3,3) size 155x13
     15      RenderBR {BR} at (0,0) size 0x13
    1416    RenderBlock {DIV} at (3,3) size 155x13 [color=#A9A9A9]
    1517      RenderText {#text} at (0,0) size 62x13
    1618        text run at (0,0) width 62: "Placeholder"
    17     RenderBlock {DIV} at (3,3) size 155x13
    18       RenderBR {BR} at (0,0) size 0x13
    1919caret: position 0 of child 0 {BR} of child 0 {DIV} of {#document-fragment} of child 1 {TEXTAREA} of child 3 {DIV} of body

  • trunk/LayoutTests/platform/mac/fast/forms/textarea-placeholder-visibility-2-expected.txt

    r139198 r155408  
    1212layer at (10,44) size 161x32 clip at (11,45) size 159x30
    1313  RenderTextControl {TEXTAREA} at (2,2) size 161x32 [bgcolor=#FFFFFF] [border: (1px solid #000000)]
     14    RenderBlock {DIV} at (3,3) size 155x13
    1415    RenderBlock {DIV} at (3,3) size 155x13 [color=#A9A9A9]
    1516      RenderText {#text} at (0,0) size 62x13
    1617        text run at (0,0) width 62: "Placeholder"
    17     RenderBlock {DIV} at (3,3) size 155x13
    1818caret: position 0 of child 0 {DIV} of {#document-fragment} of child 1 {TEXTAREA} of child 3 {DIV} of body

  • trunk/Source/WebCore/ChangeLog

    r155398 r155408  
     12013-09-09  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Remove HTMLTextFormControl::fixPlaceholderRenderer
     4        https://bugs.webkit.org/show_bug.cgi?id=121058
     5
     6        Reviewed by Kent Tamura.
     7
     8        HTMLTextFormControl::fixPlaceholderRenderer was added in r118733 to swap the order in which placeholder appears.
     9        Namely, when a text form control element is focused, placeholder should be rendered behind the text for the caret
     10        to render on top of, not behind, the placeholder text.  However, we can achieve the same effect by changing
     11        the order of elements in the shadow DOM instead of manually manipuating the render tree as the assertion failure
     12        mentioned in the change log is a bogus one.
     13
     14        Cleaned up the code by removing HTMLTextFormControl::fixPlaceholderRenderer and changed the order in which inner
     15        text element and placeholder element appear. Also fixed the assertion in Position's constructor.
     16
     17        * dom/Position.cpp:
     18        (WebCore::Position::Position): The original assertion was bogus. What we don't want have is for a Position to be
     19        before or after a shadow root. It's fine for it be anchroed against the shadow root as long as it's inside.
     20
     21        * html/HTMLTextAreaElement.cpp:
     22        (WebCore::HTMLTextAreaElement::updatePlaceholderText): Insert the placeholder before the inner text element.
     23        * html/HTMLTextAreaElement.h:
     24        * html/HTMLTextFormControlElement.cpp:
     25        * html/HTMLTextFormControlElement.h:
     26        * html/TextFieldInputType.cpp:
     27        (WebCore::TextFieldInputType::updatePlaceholderText): Ditto.
     28        * html/TextFieldInputType.h:
     29
    1302013-09-09  Andreas Kling  <akling@apple.com>
    231

  • trunk/Source/WebCore/dom/Position.cpp

    r155366 r155408  
    8686{
    8787#if ENABLE(SHADOW_DOM)
    88     ASSERT((m_anchorNode && RuntimeEnabledFeatures::shadowDOMEnabled())
    89            || !m_anchorNode || !m_anchorNode->isShadowRoot());
     88    ASSERT((m_anchorNode && RuntimeEnabledFeatures::shadowDOMEnabled()) || !m_anchorNode || !m_anchorNode->isShadowRoot() || m_anchorNode == containerNode());
    9089#else
    91     ASSERT(!m_anchorNode || !m_anchorNode->isShadowRoot());
     90    ASSERT(!m_anchorNode || !m_anchorNode->isShadowRoot() || m_anchorNode == containerNode());
    9291#endif
    9392    ASSERT(!m_anchorNode || !m_anchorNode->isPseudoElement());
    … …  
    101100{
    102101#if ENABLE(SHADOW_DOM)
    103     ASSERT((m_anchorNode && RuntimeEnabledFeatures::shadowDOMEnabled())
    104            || !m_anchorNode || !m_anchorNode->isShadowRoot());
     102    ASSERT((m_anchorNode && RuntimeEnabledFeatures::shadowDOMEnabled()) || !m_anchorNode || !m_anchorNode->isShadowRoot() || m_anchorNode == containerNode());
    105103#else
    106     ASSERT(!m_anchorNode || !m_anchorNode->isShadowRoot());
     104    ASSERT(!m_anchorNode || !m_anchorNode->isShadowRoot() || m_anchorNode == containerNode());
    107105#endif
    108106

  • trunk/Source/WebCore/html/HTMLTextAreaElement.cpp

    r154962 r155408  
    518518}
    519519
    520 void HTMLTextAreaElement::didAttachRenderers()
    521 {
    522     HTMLTextFormControlElement::didAttachRenderers();
    523     fixPlaceholderRenderer(m_placeholder, innerTextElement());
    524 }
    525 
    526520bool HTMLTextAreaElement::matchesReadOnlyPseudoClass() const
    527521{
    … …  
    548542        m_placeholder = placeholder.get();
    549543        m_placeholder->setPseudo(AtomicString("-webkit-input-placeholder", AtomicString::ConstructFromLiteral));
    550         userAgentShadowRoot()->insertBefore(m_placeholder, innerTextElement()->nextSibling(), ASSERT_NO_EXCEPTION);
     544        userAgentShadowRoot()->insertBefore(m_placeholder, innerTextElement()->nextSibling());
    551545    }
    552546    m_placeholder->setInnerText(placeholderText, ASSERT_NO_EXCEPTION);
    553     fixPlaceholderRenderer(m_placeholder, innerTextElement());
    554 }
    555 
    556 }
     547}
     548
     549}

  • trunk/Source/WebCore/html/HTMLTextAreaElement.h

    r155155 r155408  
    112112
    113113    virtual bool shouldUseInputMethod() OVERRIDE;
    114     virtual void didAttachRenderers() OVERRIDE;
    115114    virtual bool matchesReadOnlyPseudoClass() const OVERRIDE;
    116115    virtual bool matchesReadWritePseudoClass() const OVERRIDE;

  • trunk/Source/WebCore/html/HTMLTextFormControlElement.cpp

    r154877 r155408  
    169169}
    170170
    171 void HTMLTextFormControlElement::fixPlaceholderRenderer(HTMLElement* placeholder, HTMLElement* siblingElement)
    172 {
    173     // FIXME: We should change the order of DOM nodes. But it makes an assertion
    174     // failure in editing code.
    175     if (!placeholder || !placeholder->renderer())
    176         return;
    177     RenderObject* placeholderRenderer = placeholder->renderer();
    178     RenderObject* siblingRenderer = siblingElement->renderer();
    179     if (!siblingRenderer)
    180         return;
    181     if (placeholderRenderer->nextSibling() == siblingRenderer)
    182         return;
    183     RenderObject* parentRenderer = placeholderRenderer->parent();
    184     ASSERT(siblingRenderer->parent() == parentRenderer);
    185     parentRenderer->removeChild(placeholderRenderer);
    186     parentRenderer->addChild(placeholderRenderer, siblingRenderer);
    187 }
    188 
    189171void HTMLTextFormControlElement::setSelectionStart(int start)
    190172{

  • trunk/Source/WebCore/html/HTMLTextFormControlElement.h

    r154868 r155408  
    5555    virtual HTMLElement* placeholderElement() const = 0;
    5656    void updatePlaceholderVisibility(bool);
    57     static void fixPlaceholderRenderer(HTMLElement* placeholder, HTMLElement* siblingElement);
    5857
    5958    int indexForVisiblePosition(const VisiblePosition&) const;

  • trunk/Source/WebCore/html/TextFieldInputType.cpp

    r154877 r155408  
    419419        m_placeholder = HTMLDivElement::create(&element()->document());
    420420        m_placeholder->setPseudo(AtomicString("-webkit-input-placeholder", AtomicString::ConstructFromLiteral));
    421         element()->userAgentShadowRoot()->insertBefore(m_placeholder, m_container ? m_container->nextSibling() : innerTextElement()->nextSibling(), ASSERT_NO_EXCEPTION);
     421        element()->userAgentShadowRoot()->insertBefore(m_placeholder, m_container ? m_container.get() : innerTextElement(), ASSERT_NO_EXCEPTION);
    422422    }
    423423    m_placeholder->setInnerText(placeholderText, ASSERT_NO_EXCEPTION);
    424     element()->fixPlaceholderRenderer(m_placeholder.get(), m_container ? m_container.get() : m_innerText.get());
    425 }
    426 
    427 void TextFieldInputType::attach()
    428 {
    429     InputType::attach();
    430     // If container exists, the container should not have any content data.
    431     ASSERT(!m_container || !m_container->renderStyle() || !m_container->renderStyle()->hasContent());
    432 
    433     element()->fixPlaceholderRenderer(m_placeholder.get(), m_container ? m_container.get() : m_innerText.get());
    434424}
    435425

  • trunk/Source/WebCore/html/TextFieldInputType.h

    r154721 r155408  
    5858
    5959protected:
    60     virtual void attach() OVERRIDE;
    6160    virtual bool needsContainer() const;
    6261    virtual bool shouldHaveSpinButton() const;
Note: See TracChangeset for help on using the changeset viewer.