[Webkit-unassigned] [Bug 110375] Add client callbacks to notify of changes of associated from controls

[bugzilla-daemon at webkit.org]

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





--- Comment #45 from Dane Walllinga <dgwallinga at chromium.org>  2013-02-22 18:07:05 PST ---
(From update of attachment 189650)
View in context: https://bugs.webkit.org/attachment.cgi?id=189650&action=review

>> Source/WebCore/html/FormAssociatedElement.cpp:165
>> +    if (m_form && m_form != currentForm && m_form->inDocument()
> 
> We probably want the Client to have a method like bool shouldNotifyOnFormChanges() that lets you control if these are batched and if they're fired.

Sounds reasonable enough. How do you feel about the argument for moving things to ChromeClient?

>>> Source/WebCore/html/FormAssociatedElement.cpp:281
>>> +      element->document()->frame()->editor()->client()->didAssociateInput(element);
>> 
>> Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
> 
> This is going to spam you with notifications and also means you have tons of timers. This isn't the approach you want. Instead you want to collect in a HashSet (or ListHashSet) all the Elements that have changed and then give a batch notification to the client.
> 
> didAssociateFormControls(HashSet<Element>) or some such.

Assuming we still don't want to increase the size of an HTMLFormElement or formAssociatedElement, this would be a HashSet (or ListHashSet) that's kept in the document, and then presumably we'd call a method on document from somewhere in here that would be something like document()->didAssociateInput(element), which in turn keeps track of the set, and also sets its timer (or resets, if it's already set) to fire after some small number of milliseconds which will pass everything so far accumulated onto the EditorClient. Have I understood that correctly?

>>> Source/WebCore/html/FormAssociatedElement.cpp:282
>>> +    }
>> 
>> One line control clauses should not use braces.  [whitespace/braces] [4]
> 
> webkit-patch should be warning you about these before you upload. It's nice if you fix them before you upload. :)

Didn't notice any warnings from webkit-patch, but I had gotten into the habit of running check-webkit-style first. Just forgot to for this one.

>> Source/WebCore/html/FormAssociatedElement.h:122
>> +    Timer<FormAssociatedElement> m_didAssociateElementTimer;
> 
> This makes every form element bigger for just this feature which is bad. You want a shared timer per document probably.

fair enough

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