Bug 111826

Summary: RenderTextControlSingleLine should not assume that its text element has a renderer
Product: WebKit Reporter: Dominic Cooney <dominicc>
Component: Layout and RenderingAssignee: Dominic Cooney <dominicc>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, eric, esprehn+autocc, inferno, leviw, mifenton, ojan.autocc, ojan, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Dominic Cooney
Reported 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;.
Attachments
Patch (11.15 KB, patch)
2013-03-07 23:35 PST, Dominic Cooney 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Dominic Cooney
Comment 1 2013-03-07 23:35:24 PST
Created attachment 192161 [details] Patch
Ojan Vafai
Comment 2 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?
WebKit Review Bot
Comment 3 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>
WebKit Review Bot
Comment 4 2013-03-08 10:14:50 PST
All reviewed patches have been landed. Closing bug.
Christian Biesinger
Comment 5 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...
Dominic Cooney
Comment 6 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.
Dominic Cooney
Comment 7 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.
Note You need to log in before you can comment on or make changes to this bug.