Bug 111826 – RenderTextControlSingleLine should not assume that its text element has a renderer

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Dominic Cooney]

RenderTextControlSingleLine dereferences innerTextElement()->renderBox() during layout, hit testing and scrolling. However that element may not have a renderer, for example if ::-webkit-textfield-decoration-controller is used to set it to display: none;.

[Dominic Cooney]

Created attachment 192161 [details]
Patch

[Ojan Vafai]

Comment on attachment 192161 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=192161&action=review

> Source/WebCore/rendering/RenderTextControlSingleLine.cpp:176
> +        ASSERT(innerTextRenderer);

Having a placeholderBox always means you'll also have a innerTextRenderer?

[WebKit Review Bot]

Comment on attachment 192161 [details]
Patch

Clearing flags on attachment: 192161

Committed r145239: <http://trac.webkit.org/changeset/145239>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Christian Biesinger]

Comment on attachment 192161 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=192161&action=review

> Source/WebCore/rendering/RenderTextControlSingleLine.cpp:134
> +        if (innerTextRenderer)

this check is not necessary, because just above you added innerTextRenderer to the if...

[Dominic Cooney]

(In reply to comment #2)
> Having a placeholderBox always means you'll also have a innerTextRenderer?

I thought so, but now digging into TextFieldInputType::createShadowDOM I am not so sure. I will investigate.

(In reply to comment #5)
> (From update of attachment 192161 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=192161&action=review
> 
> > Source/WebCore/rendering/RenderTextControlSingleLine.cpp:134
> > +        if (innerTextRenderer)
> 
> this check is not necessary, because just above you added innerTextRenderer to the if...

Oops. Code blindness. I filed bug 112406.

[Dominic Cooney]

(In reply to comment #6)
> (In reply to comment #2)
> > Having a placeholderBox always means you'll also have a innerTextRenderer?
> 
> I thought so, but now digging into TextFieldInputType::createShadowDOM I am not so sure. I will investigate.

I have filed bug 112410 for this. I think HTMLTextFormControlElement::fixPlaceholderRenderer makes this assumption too, but the assumption is wrong.