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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Mar 5 14:00:28 PST 2013


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.

More information about the webkit-unassigned mailing list