[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 #66 from Dane Walllinga <dgwallinga at chromium.org>  2013-03-05 17:26:44 PST ---
(From update of attachment 191546)
View in context: https://bugs.webkit.org/attachment.cgi?id=191546&action=review

>> Source/WebCore/dom/Document.cpp:6057
>> +    m_didAssociateFormControlsTimer.startOneShot(0.1);
> 
> Why not 0?

done

>> Source/WebCore/html/FormAssociatedElement.cpp:166
>> +        element->document()->didAssociateFormControl(element);
> 
> What if we associated a form and then subsequently "unassociated" it?
> Why is it okay to still notify the client.
> 
> It might be okay to do that for your specific use case but that doesn't sound like a sound API to me.

You're saying we should notify the ChromeClient of unassociations? Or that if an association is followed by an unassociation within the time it takes for the Document to send changes onward, we shouldn't inform ChromeClient of anything? That later doesn't make much sense to me, as it would then be the case that what the ChromeClient gets informed of depends on how things get batched.

>> Source/WebCore/html/FormAssociatedElement.h:27
>> +#include "Timer.h"
> 
> Why do we need Timer here?

because I was lazy in cleaning up dependencies after my last large change

>> Source/WebKit/chromium/src/ChromeClientImpl.cpp:1178
>> +    WebVector<WebNode*> elementVector((size_t) elements.size());
> 
> Please use static_cast.

done

>> Source/WebKit/chromium/src/ChromeClientImpl.cpp:1182
>> +        elementVector[i] = new WebNode((*it));
> 
> I don't think this is the way to get WebNode. Simply call WebNode. r- because of this.

done

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