[webkit-reviews] review denied: [Bug 48496] Add WebKit2 API for window feature getter/setters : [Attachment 72133] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Oct 28 06:33:31 PDT 2010


John Sullivan <sullivan at apple.com> has denied Sam Weinig <sam at webkit.org>'s
request for review:
Bug 48496: Add WebKit2 API for window feature getter/setters
https://bugs.webkit.org/show_bug.cgi?id=48496

Attachment 72133: Patch
https://bugs.webkit.org/attachment.cgi?id=72133&action=review

------- Additional Comments from John Sullivan <sullivan at apple.com>
There's a mixture of naming styles here, e.g.:

WebChromeClient: setMenubarVisible/menubarVisible
WebUIClient: setMenuBarIsVisible/menuBarIsVisible
MiniBrowser/TestController: setMenuBarVisible/getMenuBarVisible

The AppKit conventions that we generally try to match are none of the above --
those conventions would specify: setMenuBarVisible (or setMenubarVisible) /
isMenuBarVisible (or isMenubarVisible).

I don't care whether the B in Menubar is capitalized or not, but we should try
to match existing style in WebKit if there are precedents. Of the other styles,
I would pick the AppKit style, but if there's some reason to use a different
style then we should use it consistently.

The content all seems fine but I think we need to get the names more consistent
so I'm giving this an r-.


 In WebChromeClient you' MiniBrowser you're using
getMenuBarVisible/setMenuBarVisible (etc.) whereas in WebUIClient you're using
getMenuBarIsVisible/setMenuBarIsVisible


More information about the webkit-reviews mailing list