[Webkit-unassigned] [Bug 153116] Need ability to specify alternate image for AutoFill button in input fields

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jan 29 09:30:35 PST 2016


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

--- Comment #29 from Darin Adler <darin at apple.com> ---
Three other small suggestions. You can probably do two of these if you come back for cleanup.

(In reply to comment #27)
> void TextFieldInputType::updateAutoFillButton()
> {
>     if (shouldDrawAutoFillButton()) {
>         if (!m_container)
>             createContainer();
> 
>         if (!m_autoFillButton) {
>             m_autoFillButton = AutoFillButtonElement::create(element().document(), *this);
>             m_container->appendChild(*m_autoFillButton, IGNORE_EXCEPTION);
>         }
> 
>         m_autoFillButton->setPseudo(autoFillButtonTypeToAutoFillButtonPseudoClassName(element().autoFillButtonType()));
>         m_autoFillButton->setInlineStyleProperty(CSSPropertyDisplay, CSSValueBlock, true);
>         return;
>     }
>     
>     if (m_autoFillButton)
>         m_autoFillButton->setInlineStyleProperty(CSSPropertyDisplay, CSSValueNone, true);        
> }

Only thing I would do differently is that I would do the "should not draw" case first as the early exit case, reversing the logic.

    if (!shouldDrawAutoFillButton()) {
        if (m_autoFillButton)
            m_autoFillButton->setInlineStyleProperty(CSSPropertyDisplay, CSSValueNone, true);
        return;
    }

That way less of the real logic of the function is nested inside an if statement.

Also, should not use IGNORE_EXCEPTION in the call to appendChild. We want the default behavior, ASSERT_NO_EXCEPTION.

I’m also not 100% sure why we have to set important to true in these style rules. It would be nice if someone double checked that we need to do that, possibly even making a test case, and if not, remove those unsightly ", true" in all the calls to setInlineStyleProperty.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160129/c25a381c/attachment.html>


More information about the webkit-unassigned mailing list