[Webkit-unassigned] [Bug 18703] changing the 'size' property on a text input does not affect its length

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Oct 24 09:26:32 PDT 2008


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


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #24640|review?(sam at webkit.org)     |review+
               Flag|                            |




------- Comment #15 from darin at apple.com  2008-10-24 09:26 PDT -------
(From update of attachment 24640)
I think the basics of this patch are great. It can land as-is.

There are two ways I think it could be improved.

The real architecture for something like this is to make the DOM element call
updateFromElement() on the renderer at the right time. This is considered
better than making a specific call to the renderer, because it's up to the
renderer to know what decisions it makes about the appearance based on what
attributes. So instead of a call to setNeedsLayoutAndPrefWidthsRecalc, we'd
have a call to updateFromElement here. Then
RenderTextControl::updateFromElement would get logic to notice changes in the
size attribute (for HTMLInputElement) and the cols attribute (for
HTMLTextAreaElement) to determine whether that will change the size. This is
better encapsulation. We want the invalidation based on size to go into
RenderText control because RenderTextControl::calcPrefWidths is the function
that knows the algorithm for how it affects the size. Unfortunately,
HTMLTextAreaElement already uses the same design you're using here, so I don't
think this change makes things any worse.

So:

    1) Use updateFromElement and put the logic about exactly what the effect of
this attribute it into RenderTextControl. And change HTMLTextAreaElement to
call updateFromElement instead.

We want to make sure to to do recalc that's unnecessary; it's common for sites
to have bugs where they repeatedly set an attribute, and we'd like to minimize
the work we do in cases like that.

    2) Don't do any update if the size isn't changing. This can be done most
elegantly inside a RenderTextControl::updateFromElement function by remembering
the old size factor value from calcPrefWidths, but it can even be done in
HTMLInputElement::parseMappedAttribute by checking the old m_size value and not
calling anything if it's not changing.

The test would be even better if the regression test said what it was testing
in the output.

Anyway, I would have loved to see the improvements above done, but the change
seems fine exactly as-is.

r=me


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



More information about the webkit-unassigned mailing list