[webkit-reviews] review denied: [Bug 68162] [chromium]The focus of an input field inside an Iframe doesn't get cleared even though clearFocusedNode is called. : [Attachment 107498] fix +unit test patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Sep 15 11:21:10 PDT 2011


Adam Barth <abarth at webkit.org> has denied chandra shekar vallala
<chandra.vallala at motorola.com>'s request for review:
Bug 68162: [chromium]The focus of an input field inside an Iframe doesn't get
cleared even though clearFocusedNode is called.
https://bugs.webkit.org/show_bug.cgi?id=68162

Attachment 107498: fix +unit test patch
https://bugs.webkit.org/attachment.cgi?id=107498&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=107498&action=review


Thanks for the patch.  This change looks beneficial, there are just a couple
process-oriented nits to clear up and then we'll be all set.

> Source/WebKit/chromium/src/WebViewImpl.cpp:1783
> +    // Get the focused frame.

This comment doesn't explain the "why" of any code, just the "what", which we
can read on the next line anyway.  It's probably best to remove.

> Source/WebKit/chromium/ChangeLog:3
> +	   Need a short description and bug URL (OOPS!)

Please fill out this information.  The Tools/Scripts/prepare-Changelog script
can do that for you if you use the "-b" option.  Alternatively, "webkit-patch
upload" can also help make this easier.

> Source/WebKit/chromium/ChangeLog:6
> +

It's a good idea to add some text to the ChangeLog explaining why you're making
this change.  That helps folks reviewing your patch understand that problem
you're solving and it helps other folks looking at this change in the future
understand what you're accomplishing.


More information about the webkit-reviews mailing list