Replace HTMLTextFormControlElement::fixPlaceholderRenderer with an override of RenderTextControl::a…

Replace HTMLTextFormControlElement::fixPlaceholderRenderer with an override of RenderTextControl::addChild

fixPlaceholderRenderer was used to rearrange the render tree to make sure that
the placeholder text was always first. It required a synchronous attach hook though
so isntead just teach RenderTextControl::addChild how to do this reordering. The hack
is still really ugly, but at least it's contained in one place.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=156619

[eseidel, 2013-08-20 21:38:29]

lgtm

[commit-bot: I haz the power, 2013-08-20 21:38:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/esprehn@chromium.org/23035007/1

[commit-bot: I haz the power, 2013-08-21 05:18:33]

Retried try job too often on win_blink_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_blink_...

[esprehn, 2013-08-22 03:53:42]

This doesn't work right now because there's tests for what happens when you
reproject the use agent shadow of an <input> into a user shadow and messing with
::addChild breaks that. I don't think we're ever going to enable that feature
since it leaks implementation details and leads to security bugs. We should
disable reprojecting UA shadow root for all elements, it's not clear why we only
disabled it for <details>, it's never safe to do it.

[tkent, 2013-08-22 03:59:06]

On 2013/08/22 03:53:42, esprehn wrote:
> This doesn't work right now because there's tests for what happens when you
> reproject the use agent shadow of an <input> into a user shadow and messing
with
> ::addChild breaks that. I don't think we're ever going to enable that feature
> since it leaks implementation details and leads to security bugs. We should
> disable reprojecting UA shadow root for all elements, it's not clear why we
only
> disabled it for <details>, it's never safe to do it.

Yeah, the test is wrong for our consensus.  You may mark the test fail and go
ahead this CL.

[ojan, 2013-08-22 18:41:47]

lgtm

https://codereview.chromium.org/23035007/diff/1/Source/core/html/TextFieldInp...
File Source/core/html/TextFieldInputType.cpp (right):

https://codereview.chromium.org/23035007/diff/1/Source/core/html/TextFieldInp...
Source/core/html/TextFieldInputType.cpp:419:
element()->userAgentShadowRoot()->insertBefore(m_placeholder, m_container ?
m_container->nextSibling() : innerTextElement()->nextSibling(),
ASSERT_NO_EXCEPTION);
Can you make this AttachLazily as part of this patch? Or I can do it in a
followup.

[esprehn, 2013-08-23 01:41:57]

On 2013/08/22 18:41:47, ojan wrote:
> lgtm
> 
>
https://codereview.chromium.org/23035007/diff/1/Source/core/html/TextFieldInp...
> File Source/core/html/TextFieldInputType.cpp (right):
> 
>
https://codereview.chromium.org/23035007/diff/1/Source/core/html/TextFieldInp...
> Source/core/html/TextFieldInputType.cpp:419:
> element()->userAgentShadowRoot()->insertBefore(m_placeholder, m_container ?
> m_container->nextSibling() : innerTextElement()->nextSibling(),
> ASSERT_NO_EXCEPTION);
> Can you make this AttachLazily as part of this patch? Or I can do it in a
> followup.

You made it the default in a later patch, so it's cleaned up now.

[commit-bot: I haz the power, 2013-08-23 01:44:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/esprehn@chromium.org/23035007/18001

[commit-bot: I haz the power, 2013-08-23 03:06:06]

Retried try job too often on linux_blink_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[commit-bot: I haz the power, 2013-08-23 03:47:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/esprehn@chromium.org/23035007/36001

[commit-bot: I haz the power, 2013-08-23 05:43:35]

Change committed as 156619