Summary: | Rename WebColorChooserProxy | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ruth Fong <ruthiecftg> | ||||||||||
Component: | Forms | Assignee: | 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
Ruth Fong
2013-07-23 16:52:09 PDT
Created attachment 207410 [details]
Patch
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 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 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.
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? (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? 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. Created attachment 207412 [details]
Patch
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 Created attachment 207417 [details]
Patch
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. Created attachment 207422 [details]
Patch
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. Comment on attachment 207422 [details] Patch Clearing flags on attachment: 207422 Committed r153108: <http://trac.webkit.org/changeset/153108> All reviewed patches have been landed. Closing bug. |