[Webkit-unassigned] [Bug 82566] RenderTextControlSingleLine::scrollWidth/H/L/T should not call back to DOM tree

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=82566





--- Comment #6 from Tien-Ren Chen <trchen at chromium.org>  2012-04-09 16:10:58 PST ---
(In reply to comment #4)
> (From update of attachment 134486 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=134486&action=review
> 
> > I'm not really certain if it is the right thing to do, since it changes the layout behavior,
> 
> The tests are passing on cr-linux EWS so how come it changes the layout? What kind of changes do you see? It sounds like you should add a test that shows this difference? A test would also help us see the issue and advise a better way to fix it (I think your approach is sane but would love to see the incriminating test case).

I didn't phrase it right. What I meant to say is that it "might" change layout behavior because we no longer request the latest layout from Element. As the bots are all green I guess I worried too much. 

> > Source/WebCore/rendering/RenderTextControlSingleLine.cpp:694
> > +    if (innerTextElement() && innerTextElement()->renderBox())
> 
> I wonder if we need any of the NULL-checks. Does it make sense to have a RenderTextControlSingleLine without an inner element? (the code is not consistent about those unfortunately)
> 
> I would have to look at this logic but tkent@ may know from the top of his head.
> > Source/WebCore/rendering/RenderTextControlSingleLine.cpp:695
> > +        return innerTextElement()->renderBox()->scrollWidth();
> 
> As you pointed out, there is a difference with respect to zoom factor between Element::scrollWidth and RenderBox::scrollWidth. To be truly accurate here, we should take the innerTextElement()'s zoom factor into account in the same way Element::scrollWidth() does. I do fear code duplication here but I don't see a good way around that as we don't want to expose the inner gut of Element::scrollWidth to everyone.

For real I have no idea what is going on(apply no idea dog meme here). Anyway the newly uploaded patch mimic the original behavior as much as possible, and we will go from there?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.