Bug 119025

Summary: Rename WebColorChooserProxy
Product: WebKit Reporter: Ruth Fong <ruthiecftg>
Component: FormsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abecsi, beidson, cdumez, cgarcia, cmarcelo, commit-queue, eflews.bot, gustavo, gyuyoung.kim, jonlee, luiz, menard, mrobinson, noam, rakuco, ruthiecftg, webkit-bug-importer, webkit-ews, zeno
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 61276    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Ruth Fong
Reported 2013-07-23 16:52:09 PDT
WebColorChooserProxy, which represents the color picker UI for an <input type='color'> element, suggests a one-to-one relationship with the WebColorChooser object. In bug 61276, the architecture will change so that only one WebColorChooserProxy object exists for a given WebPageProxy, while there can be multiple WebColorChooser objects living in the WebProcess. WebColorChooserProxy should be renamed to something like WebColorPickerUI that reflects its representation of the color picker UI.
Attachments
Patch (28.65 KB, patch)
2013-07-24 14:08 PDT, Ruth Fong 		no flags
DetailsFormatted DiffDiff
Patch (54.79 KB, patch)
2013-07-24 14:52 PDT, Ruth Fong 		no flags
DetailsFormatted DiffDiff
Patch (54.48 KB, patch)
2013-07-24 16:07 PDT, Ruth Fong 		no flags
DetailsFormatted DiffDiff
Patch (54.44 KB, patch)
2013-07-24 17:02 PDT, Ruth Fong 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2013-07-23 16:53:07 PDT
<rdar://problem/14528039>
Ruth Fong
Comment 2 2013-07-24 14:08:22 PDT
Created attachment 207410 [details] Patch
Early Warning System Bot
Comment 3 2013-07-24 14:12:56 PDT
Comment on attachment 207410 [details] Patch Attachment 207410 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/1200027
EFL EWS Bot
Comment 4 2013-07-24 14:15:04 PDT
Comment on attachment 207410 [details] Patch Attachment 207410 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/1205034
Brady Eidson
Comment 5 2013-07-24 14:16:18 PDT
Comment on attachment 207410 [details] Patch Should change it in all the build systems. `grep -r WebColorChooserProxy.cpp <path/to/WebKit2>` should find everywhere you need to replace it.
Tim Horton
Comment 6 2013-07-24 14:17:11 PDT
Comment on attachment 207410 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=207410&action=review > Source/WebKit2/UIProcess/PageClient.h:190 > + virtual PassRefPtr<WebColorPickerUI> createColorChooserProxy(WebPageProxy*, const WebCore::Color& initialColor, const WebCore::IntRect&) = 0; This method is poorly named now, isn't it?
Brady Eidson
Comment 7 2013-07-24 14:25:06 PDT
(In reply to comment #6) > (From update of attachment 207410 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=207410&action=review > > > Source/WebKit2/UIProcess/PageClient.h:190 > > + virtual PassRefPtr<WebColorPickerUI> createColorChooserProxy(WebPageProxy*, const WebCore::Color& initialColor, const WebCore::IntRect&) = 0; > > This method is poorly named now, isn't it? And is there any advantage to calling the new object WebColorPickerUI instead of WebColorPicker, considering that the abstract concept of a WebColorPicker encapsulates more than just its UI?
Ruth Fong
Comment 8 2013-07-24 14:42:05 PDT
Comment on attachment 207410 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=207410&action=review >>> Source/WebKit2/UIProcess/PageClient.h:190 >>> + virtual PassRefPtr<WebColorPickerUI> createColorChooserProxy(WebPageProxy*, const WebCore::Color& initialColor, const WebCore::IntRect&) = 0; >> >> This method is poorly named now, isn't it? > > And is there any advantage to calling the new object WebColorPickerUI instead of WebColorPicker, considering that the abstract concept of a WebColorPicker encapsulates more than just its UI? I think that the WebColorPicker(UI) object does represent UI representation of the color picker, whereas the WebColorChooser is the abstract representation for the <input type='color'> object.
Ruth Fong
Comment 9 2013-07-24 14:52:56 PDT
Created attachment 207412 [details] Patch
WebKit Commit Bot
Comment 10 2013-07-24 14:56:06 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Ruth Fong
Comment 11 2013-07-24 16:07:03 PDT
Created attachment 207417 [details] Patch
Brady Eidson
Comment 12 2013-07-24 16:55:55 PDT
Comment on attachment 207417 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=207417&action=review > Source/WebKit2/ChangeLog:10 > + Rename WebColorChooserProxy to WebColorPicker since the name WebColorChoooserProxy > + implies a one-to-one relationship with WebColorChooser, which will not be true when > + the patch for bug 61276 lands. Nitpick - It's not even true *now* Yes, the WebProcess has 1 WebColorChooser, and the UIProcess has 1 WebColorChooserProxy... but they aren't actually connected. The Object/ObjectProxy naming scheme is for a pair of objects that represent the identical object/concept on both sides of the IPC connection, so tightly intertwined that they message each other directly.
Ruth Fong
Comment 13 2013-07-24 17:02:10 PDT
Created attachment 207422 [details] Patch
WebKit Commit Bot
Comment 14 2013-07-24 17:03:46 PDT
Comment on attachment 207422 [details] Patch Rejecting attachment 207422 [details] from commit-queue. ruthiecftg@gmail.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights.
WebKit Commit Bot
Comment 15 2013-07-24 18:07:17 PDT
Comment on attachment 207422 [details] Patch Clearing flags on attachment: 207422 Committed r153108: <http://trac.webkit.org/changeset/153108>
WebKit Commit Bot
Comment 16 2013-07-24 18:07:23 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.