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

[bugzilla-daemon at webkit.org]

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


gwilson at google.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED




------- Comment #19 from gwilson at google.com  2008-11-07 10:47 PDT -------
(In reply to comment #15)
>     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.

I've tried out some of the improvements above, and found two different
possibilities for changing RenderTextControl that I'd like your opinions about.
 Both seem to work, but they both have inefficiencies that I'm not ecstatic
about....perhaps you can spot improvements?

1)  Adding size-change detection to HtmlInputElement and detection of size
changes to updateFromElement, and if the sizes have changed, calling
setNeedsLayoutAndPrefWidthsRecalc:

Added to RenderTextControl::updateFromElement:
    int old_min_width = m_minPrefWidth;
    int old_max_width = m_maxPrefWidth;
    calcPrefWidths();
    if (old_min_width != m_minPrefWidth &&
        old_max_width != m_maxPrefWidth)
        this->setNeedsLayoutAndPrefWidthsRecalc();

Added to HtmlInputElement::parseMappedAttribute:
        short old_size = m_size;
        m_size = !attr->isNull() ? attr->value().toInt() : 20;
        if (renderer() && m_size != old_size) {
            setValueMatchesRenderer(false);
            static_cast<RenderTextControl*>(renderer())->updateFromElement();
        }

I don't like this because it measures the size twice and could lead to two
calls to calcPrefWidths(), which is not efficient.  Is there a way in
RenderTextControl to detect size changes without calculating calcPrefWidths?

2)  Adding another method to RenderTextControl that is only called when widths
change, which circumvents needing to detect size changes in RenderTextControl:

Added to RenderTextControl:

void RenderTextControl::updateSizeFromElement() 
{
    setNeedsLayoutAndPrefWidthsRecalc();
    updateFromElement();
}

Added to HtmlInputElement::parseMappedAttribute:
        short old_size = m_size;
        m_size = !attr->isNull() ? attr->value().toInt() : 20;
        if (renderer() && m_size != old_size) {
            setValueMatchesRenderer(false);
           
static_cast<RenderTextControl*>(renderer())->updateSizeFromElement();
        }

I like the second option better, because it feels cleaner. However, adding
another method to RenderTextControl to be called just when the object's size
changes may not be ideal.  What can be done to improve this?


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