Bug 111826 - RenderTextControlSingleLine should not assume that its text element has a renderer
Summary: RenderTextControlSingleLine should not assume that its text element has a ren...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dominic Cooney
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-07 23:32 PST by Dominic Cooney
Modified: 2013-03-14 23:31 PDT (History)
10 users (show)

See Also:

Attachments
Patch (11.15 KB, patch)
2013-03-07 23:35 PST, Dominic Cooney 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Dominic Cooney 2013-03-07 23:32:54 PST 
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;.
Comment 1 Dominic Cooney 2013-03-07 23:35:24 PST 
Created attachment 192161 [details]
Patch
Comment 2 Ojan Vafai 2013-03-08 10:09:36 PST 
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?
Comment 3 WebKit Review Bot 2013-03-08 10:14:46 PST 
Comment on attachment 192161 [details]
Patch

Clearing flags on attachment: 192161

Committed r145239: <http://trac.webkit.org/changeset/145239>
Comment 4 WebKit Review Bot 2013-03-08 10:14:50 PST 
All reviewed patches have been landed.  Closing bug.
Comment 5 Christian Biesinger 2013-03-13 14:26:29 PDT 
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...
Comment 6 Dominic Cooney 2013-03-14 22:53:35 PDT 
(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.
Comment 7 Dominic Cooney 2013-03-14 23:31:30 PDT 
(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.