[Webkit-unassigned] [Bug 32077] textarea grows when you type

[bugzilla-daemon at webkit.org]

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





--- Comment #9 from MORITA Hajime <morrita at google.com>  2010-03-08 22:33:15 PST ---
Thank you for the feedback! I've updated the patch.

> The LayoutTests change log need not contain a description of the fix.
Fix to put test description instead.

> 
> > +<script src="./resources/textarea-percentage-dimensions.js"></script>
> 
> You can omit the “./”.
Fixed.

> Typo: “pecent”.
(...)
> Typo: “propety”. I don’t understand “and family”.
Fixed. I'm sorry for that...

> I think the description could be improved. The main points, as I see them, are
> the specific case where percentage height calculation depends on the state of
> ancestors during their layout, and the fact that by definition, the height of
> the layout root must not change during layout, or else it wouldn’t be a valid
> layout root.
I rewrote the description on ChangeLog. Thank you for your suggestion!

> 
> > +    bool isLayoutRoot = node() && view()->frameView() && view()->frameView()->layoutRoot(true) == this;
> 
> Using a local variable for this seems redundant. Why the check for node()?
Oops. The conditional was just copied from RenderBox::calcWidth(). I removed
redundant check against node().
And I also moved layout-root check from RenderBox::calcWidth() to
RenderBlock::layoutBlock() to share the criteria.
between the check for calcHeight().

> 
> >      int oldHeight = height();
> > -    calcHeight();
> > +
> > +    if (isLayoutRoot)
> > +        setHeight(previousHeight);
> > +    else
> > +        calcHeight();
> > +
> >      if (oldHeight != height()) {
> 
> This is suboptimal. If you skip the height calculation, it seems like you could
> skip the (oldHeight != height()) test and the previousHeight != height() test
> that follows. Those could be in the else block with calcHeight().
Fixed. 

Would you review the patch again?

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