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

[bugzilla-daemon at webkit.org]

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





--- Comment #94 from Dane Walllinga <dgwallinga at chromium.org>  2013-03-21 14:45:30 PST ---
(From update of attachment 194095)
View in context: https://bugs.webkit.org/attachment.cgi?id=194095&action=review

>> Source/WebCore/dom/Document.cpp:6078
>> +        || !frame()->page()->chrome()->client()->shouldNotifyOnFormChanges())
> 
> Why are you wrapping the line here? We can fit both lines in one line.

Ok - I couldn't find anything in the style guide about line length restrictions.

>> Source/WebCore/dom/Document.cpp:6090
>> +    HashSet<Element*> associatedFormControls;
> 
> Adding a blank line here will improve the readability.

done

>> Source/WebCore/dom/Document.cpp:6094
>> +    frame()->page()->chrome()->client()->didAssociateFormControls(associatedFormControls);
> 
> Now I'm reading this code for the second time, I feel like we should just pass a Vector to didAssociateFormControls because
> then we can use copyToVector in HashSet.h as opposed to manually iterating over them.

sounds good to me

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