[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


Ryosuke Niwa <rniwa at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #191546|review?                     |review-
               Flag|                            |




--- Comment #64 from Ryosuke Niwa <rniwa at webkit.org>  2013-03-05 14:02:51 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?

> Source/WebCore/html/FormAssociatedElement.cpp:166
> +    if (m_form && m_form != currentForm && m_form->inDocument())
> +        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.

> Source/WebCore/html/FormAssociatedElement.h:27
> +#include "Timer.h"

Why do we need Timer here?

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

Please use static_cast.

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

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