Changeset 118733 in webkit


Ignore:
Timestamp:
May 29, 2012 1:23:17 AM (12 years ago)
Author:
tkent@chromium.org
Message:

REGRESSION (r90971): the cursor is painted “behind” the placeholder text
​https://bugs.webkit.org/show_bug.cgi?id=87155

Reviewed by Hajime Morita.

Source/WebCore:

This regression happened only on platforms on which
RenderTheme::shouldShowPlaceholderWhenFocused() returns true.

Because the order of renderers for the editable node and the placeholder
node was:

  • A renderer for the editable node
  • A renderer for the placeholder node,

The text caret was painted, then the palceholder was painted.

We should not use z-index in the built-in shadow nodes. So the patch
fixes this bug by re-ordering these renderers.

Tests: fast/forms/input-placeholder-paint-order-2.html

fast/forms/input-placeholder-paint-order.html
fast/forms/textarea/textarea-placeholder-paint-order-2.html
fast/forms/textarea/textarea-placeholder-paint-order.html

  • html/HTMLTextFormControlElement.cpp:

(WebCore::HTMLTextFormControlElement::fixPlaceholderRenderer):
Added. Reorder the order of renderers so that the placeholder renderer
precedes the inner text renderer.

  • html/HTMLTextFormControlElement.h: Add fixPlaceholderRenderer() declaration.
  • html/HTMLTextAreaElement.cpp:

(WebCore::HTMLTextAreaElement::attach): Calls fixPlaceholderRenderer().
(WebCore::HTMLTextAreaElement::updatePlaceholderText):
ditto. Also, use innerTextElement() to improvde code readability.

  • html/HTMLTextAreaElement.h:

(HTMLTextAreaElement): Overrides attach().

  • html/TextFieldInputType.cpp:

(WebCore::TextFieldInputType::updatePlaceholderText):
Calls fixPlaceholderRenderer().
(WebCore::TextFieldInputType::attach): ditto.

  • html/TextFieldInputType.h:

(TextFieldInputType): Overrides attach().

LayoutTests:

  • fast/forms/input-placeholder-paint-order-2-expected.html: Added.
  • fast/forms/input-placeholder-paint-order-2.html: Added.
  • fast/forms/input-placeholder-paint-order.html: Added.
  • fast/forms/textarea/textarea-placeholder-paint-order-2-expected.html: Added.
  • fast/forms/textarea/textarea-placeholder-paint-order-2.html: Added.
  • fast/forms/textarea/textarea-placeholder-paint-order.html: Added.
  • platform/chromium-mac-snowleopard/fast/forms/input-placeholder-paint-order-expected.png: Added.
  • platform/chromium-mac-snowleopard/fast/forms/input-placeholder-paint-order-expected.txt: Added.
  • platform/chromium-mac-snowleopard/fast/forms/textarea/textarea-placeholder-paint-order-expected.png: Added.
  • platform/chromium-mac-snowleopard/fast/forms/textarea/textarea-placeholder-paint-order-expected.txt: Added.
  • platform/chromium/test_expectations.txt:
Location:
trunk
Files:
12 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r118730 r118733  
     12012-05-29  Kent Tamura  <tkent@chromium.org>
     2
     3        REGRESSION (r90971): the cursor is painted “behind” the placeholder text
     4        https://bugs.webkit.org/show_bug.cgi?id=87155
     5
     6        Reviewed by Hajime Morita.
     7
     8        * fast/forms/input-placeholder-paint-order-2-expected.html: Added.
     9        * fast/forms/input-placeholder-paint-order-2.html: Added.
     10        * fast/forms/input-placeholder-paint-order.html: Added.
     11        * fast/forms/textarea/textarea-placeholder-paint-order-2-expected.html: Added.
     12        * fast/forms/textarea/textarea-placeholder-paint-order-2.html: Added.
     13        * fast/forms/textarea/textarea-placeholder-paint-order.html: Added.
     14        * platform/chromium-mac-snowleopard/fast/forms/input-placeholder-paint-order-expected.png: Added.
     15        * platform/chromium-mac-snowleopard/fast/forms/input-placeholder-paint-order-expected.txt: Added.
     16        * platform/chromium-mac-snowleopard/fast/forms/textarea/textarea-placeholder-paint-order-expected.png: Added.
     17        * platform/chromium-mac-snowleopard/fast/forms/textarea/textarea-placeholder-paint-order-expected.txt: Added.
     18        * platform/chromium/test_expectations.txt:
     19
    1202012-05-29  János Badics  <jbadics@inf.u-szeged.hu>
    221

  • trunk/LayoutTests/platform/chromium/test_expectations.txt

    r118695 r118733  
    34373437BUGWK80531 MAC : fast/table/colspanMinWidth-vertical.html = IMAGE+TEXT
    34383438
     3439// New tests.  Need expectations.
     3440BUGWK87155 : fast/forms/input-placeholder-paint-order.html = MISSING FAIL
     3441BUGWK87155 : fast/forms/textarea/textarea-placeholder-paint-order.html = MISSING FAIL
     3442
    34393443// Flaky/crashing on a single platform every 30 tries or so
    34403444BUGWK82230 LINUX DEBUG : http/tests/cache/post-redirect-get.php = PASS CRASH

  • trunk/Source/WebCore/ChangeLog

    r118726 r118733  
     12012-05-29  Kent Tamura  <tkent@chromium.org>
     2
     3        REGRESSION (r90971): the cursor is painted “behind” the placeholder text
     4        https://bugs.webkit.org/show_bug.cgi?id=87155
     5
     6        Reviewed by Hajime Morita.
     7
     8        This regression happened only on platforms on which
     9        RenderTheme::shouldShowPlaceholderWhenFocused() returns true.
     10
     11        Because the order of renderers for the editable node and the placeholder
     12        node was:
     13         - A renderer for the editable node
     14         - A renderer for the placeholder node,
     15        The text caret was painted, then the palceholder was painted.
     16
     17        We should not use z-index in the built-in shadow nodes. So the patch
     18        fixes this bug by re-ordering these renderers.
     19
     20        Tests: fast/forms/input-placeholder-paint-order-2.html
     21               fast/forms/input-placeholder-paint-order.html
     22               fast/forms/textarea/textarea-placeholder-paint-order-2.html
     23               fast/forms/textarea/textarea-placeholder-paint-order.html
     24
     25        * html/HTMLTextFormControlElement.cpp:
     26        (WebCore::HTMLTextFormControlElement::fixPlaceholderRenderer):
     27        Added. Reorder the order of renderers so that the placeholder renderer
     28        precedes the inner text renderer.
     29        * html/HTMLTextFormControlElement.h: Add fixPlaceholderRenderer() declaration.
     30
     31        * html/HTMLTextAreaElement.cpp:
     32        (WebCore::HTMLTextAreaElement::attach): Calls fixPlaceholderRenderer().
     33        (WebCore::HTMLTextAreaElement::updatePlaceholderText):
     34        ditto. Also, use innerTextElement() to improvde code readability.
     35        * html/HTMLTextAreaElement.h:
     36        (HTMLTextAreaElement): Overrides attach().
     37
     38        * html/TextFieldInputType.cpp:
     39        (WebCore::TextFieldInputType::updatePlaceholderText):
     40        Calls fixPlaceholderRenderer().
     41        (WebCore::TextFieldInputType::attach): ditto.
     42        * html/TextFieldInputType.h:
     43        (TextFieldInputType): Overrides attach().
     44
    1452012-05-28  Kentaro Hara  <haraken@chromium.org>
    246

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

    r118477 r118733  
    497497}
    498498
     499void HTMLTextAreaElement::attach()
     500{
     501    HTMLTextFormControlElement::attach();
     502    fixPlaceholderRenderer(m_placeholder.get(), innerTextElement());
     503}
     504
    499505void HTMLTextAreaElement::updatePlaceholderText()
    500506{
    … …  
    512518        m_placeholder = HTMLDivElement::create(document());
    513519        m_placeholder->setShadowPseudoId("-webkit-input-placeholder");
    514         shadow()->oldestShadowRoot()->insertBefore(m_placeholder, shadow()->oldestShadowRoot()->firstChild()->nextSibling(), ec);
     520        shadow()->oldestShadowRoot()->insertBefore(m_placeholder, innerTextElement()->nextSibling(), ec);
    515521        ASSERT(!ec);
    516522    }
    517523    m_placeholder->setInnerText(placeholderText, ec);
    518524    ASSERT(!ec);
    519 }
    520 
    521 }
     525    fixPlaceholderRenderer(m_placeholder.get(), innerTextElement());
     526}
     527
     528}

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

    r117195 r118733  
    110110
    111111    virtual bool shouldUseInputMethod();
     112    virtual void attach() OVERRIDE;
    112113
    113114    bool valueMissing(const String& value) const { return isRequiredFormControl() && !disabled() && !readOnly() && value.isEmpty(); }

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

    r118192 r118733  
    167167}
    168168
     169void HTMLTextFormControlElement::fixPlaceholderRenderer(HTMLElement* placeholder, HTMLElement* siblingElement)
     170{
     171    // FIXME: We should change the order of DOM nodes. But it makes an assertion
     172    // failure in editing code.
     173    if (!placeholder || !placeholder->renderer())
     174        return;
     175    RenderObject* placeholderRenderer = placeholder->renderer();
     176    RenderObject* siblingRenderer = siblingElement->renderer();
     177    ASSERT(siblingRenderer);
     178    if (placeholderRenderer->nextSibling() == siblingRenderer)
     179        return;
     180    RenderObject* parentRenderer = placeholderRenderer->parent();
     181    ASSERT(parentRenderer == siblingRenderer->parent());
     182    parentRenderer->removeChild(placeholderRenderer);
     183    parentRenderer->addChild(placeholderRenderer, siblingRenderer);
     184}
     185
    169186RenderTextControl* HTMLTextFormControlElement::textRendererAfterUpdateLayout()
    170187{

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

    r118192 r118733  
    5555    virtual HTMLElement* placeholderElement() const = 0;
    5656    void updatePlaceholderVisibility(bool);
     57    static void fixPlaceholderRenderer(HTMLElement* placeholder, HTMLElement* siblingElement);
    5758
    5859    int indexForVisiblePosition(const VisiblePosition&) const;

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

    r116730 r118733  
    411411    m_placeholder->setInnerText(placeholderText, ec);
    412412    ASSERT(!ec);
     413    element()->fixPlaceholderRenderer(m_placeholder.get(), m_container ? m_container.get() : m_innerText.get());
     414}
     415
     416void TextFieldInputType::attach()
     417{
     418    InputType::attach();
     419    element()->fixPlaceholderRenderer(m_placeholder.get(), m_container ? m_container.get() : m_innerText.get());
    413420}
    414421

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

    r112839 r118733  
    8181    virtual void updatePlaceholderText() OVERRIDE;
    8282    virtual bool appendFormData(FormDataList&, bool multipart) const OVERRIDE;
     83    virtual void attach() OVERRIDE;
    8384
    8485    RefPtr<HTMLElement> m_container;
Note: See TracChangeset for help on using the changeset viewer.