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
+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.
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
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
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
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
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]
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
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
+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.
, 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());
}
, 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());
return m_placeholder;
}
-void HTMLTextAreaElement::didAttachRenderers()
-{
- HTMLTextFormControlElement::didAttachRenderers();
- fixPlaceholderRenderer(m_placeholder, innerTextElement());
-}
-
bool HTMLTextAreaElement::matchesReadOnlyPseudoClass() const
{
return isReadOnly();
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());
}
}
virtual void accessKeyAction(bool sendMouseEvents);
virtual bool shouldUseInputMethod() OVERRIDE;
- virtual void didAttachRenderers() OVERRIDE;
virtual bool matchesReadOnlyPseudoClass() const OVERRIDE;
virtual bool matchesReadWritePseudoClass() const OVERRIDE;
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());
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;
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
#endif
protected:
- virtual void attach() OVERRIDE;
virtual bool needsContainer() const;
virtual bool shouldHaveSpinButton() const;
virtual void createShadowSubtree() OVERRIDE;