<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Need ability to specify alternate image for AutoFill button in input fields"
href="https://bugs.webkit.org/show_bug.cgi?id=153116#c18">Comment # 18</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Need ability to specify alternate image for AutoFill button in input fields"
href="https://bugs.webkit.org/show_bug.cgi?id=153116">bug 153116</a>
from <span class="vcard"><a class="email" href="mailto:zacharyli323@gmail.com" title="Zach Li <zacharyli323@gmail.com>"> <span class="fn">Zach Li</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=269302&action=diff" name="attach_269302" title="Patch v2">attachment 269302</a> <a href="attachment.cgi?id=269302&action=edit" title="Patch v2">[details]</a></span>
Patch v2
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=269302&action=review">https://bugs.webkit.org/attachment.cgi?id=269302&action=review</a>
<span class="quote">>>> Source/WebCore/html/HTMLInputElement.h:436
>>> + AutoFillButtonType m_autoFillButtonType;
>>
>> This is only one bit. It’s not great that it’s going to make every HTMLInputElement 32 bits bigger (or more). Notice that all the fields around this are using bitfields to keep things cheap. Not OK to make every input element so much bigger. I think this should be:
>>
>> unsigned m_autoFillButtonType : 1; // AutoFillButtonType
>>
>> To match how we do things in the rest of the class.
>>
>> We could also consider combining this with
>
> We could also consider combining this with the flag that says there is an auto-fill button. We don’t need to support setting the type when showAutoFillButton is false, and over time that could save us some bits.</span >
Great suggestion! The new AutoFill button type will contain Contacts, Credentials, and Uninitialized. The Uninitialized is equivalent to the AutoFill button being not enabled.
<span class="quote">>> Source/WebCore/html/HTMLTextFormControlElement.h:37
>> +enum class AutoFillButtonType : uint32_t { Credentials, AddressBook };
>
> 32 bits just to hold one bit? How about 8 bits?</span >
Will change to uint8_t.
<span class="quote">>> Source/WebCore/html/InputType.h:268
>> + virtual void updateAutoFillButton(AutoFillButtonType);
>
> We should not add this argument to this function. The members functions in TextFieldInputType can get the type from the element directly; it doesn’t need to be passed in.</span >
You are right, I can get the type from element().autoFillButtonType().
<span class="quote">>>> Source/WebCore/html/TextFieldInputType.cpp:644
>>> + createAutoFillButton(autoFillButtonType);
>>
>> This looks wrong. If the auto fill button type has changed, I don’t see code to cause the old one to be destroyed and a new one of the correct type to be created.
>
> We should make a test case that shows the auto-fill button correctly changing even when the attribute is changed after the fact.</span >
Ideally, the case should not happen, but I will handle such case.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>