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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Mar 8 10:54:14 PST 2010


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


mitz at webkit.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #50203|review?                     |review-
               Flag|                            |




--- Comment #7 from mitz at webkit.org  2010-03-08 10:54:14 PST ---
(From update of attachment 50203)
> +++ b/LayoutTests/ChangeLog
> @@ -1,3 +1,32 @@
> +2010-03-08  MORITA Hajime  <morrita at google.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Textarea grows when you type.
> +        https://bugs.webkit.org/show_bug.cgi?id=32077
> +
> +        Fix to prevent box height change on re-layout with the renderer of
> +        the box as a root. RenderBlock with pecent-specified 'height'
> +        propety (and family) did cause different box height values between
> +        full-layout and partial layout with the renderer as a layout-root.
> +        This fix restores original height in such case.

The LayoutTests change log need not contain a description of the fix.

> +<script src="./resources/textarea-percentage-dimensions.js"></script>

You can omit the “./”.

> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,29 @@
> +2010-03-08  MORITA Hajime  <morrita at google.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Textarea grows when you type.
> +        https://bugs.webkit.org/show_bug.cgi?id=32077
> +
> +        Fix to prevent box height change on re-layout with the renderer of
> +        the box as a root. RenderBlock with pecent-specified 'height'

Typo: “pecent”.

> +        propety (and family) did cause different box height values between

Typo: “propety”. I don’t understand “and family”.

> +        full-layout and partial layout with the renderer as a layout-root.
> +        This fix restores original height in such case.

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.

> +    bool isLayoutRoot = node() && view()->frameView() && view()->frameView()->layoutRoot(true) == this;

Using a local variable for this seems redundant. Why the check for node()?

>      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().

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


More information about the webkit-unassigned mailing list