Bug 48496

Summary: Add WebKit2 API for window feature getter/setters
Product: WebKit Reporter: Sam Weinig <sam>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: webkit-ews, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
sullivan: review-
Updated. andersca: review+

Description Sam Weinig 2010-10-27 20:10:22 PDT 
Add WebKit2 API for window feature getter/setters
Comment 1 Sam Weinig 2010-10-27 20:12:00 PDT 
Created attachment 72133 [details]
Patch
Comment 2 WebKit Review Bot 2010-10-27 20:13:32 PDT 
Attachment 72133 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/UIProcess/API/C/WKPage.h:172:  Extra space between WKPageGetToolbarsAreVisibleCallback and toolbarsAreVisible  [whitespace/declaration] [3]
WebKit2/UIProcess/API/C/WKPage.h:173:  Extra space between WKPageSetToolbarsAreVisibleCallback and setToolbarsAreVisible  [whitespace/declaration] [3]
WebKit2/UIProcess/API/C/WKPage.h:174:  Extra space between WKPageGetMenuBarIsVisibleCallback and menuBarIsVisible  [whitespace/declaration] [3]
WebKit2/UIProcess/API/C/WKPage.h:175:  Extra space between WKPageSetMenuBarIsVisibleCallback and setMenuBarIsVisible  [whitespace/declaration] [3]
WebKit2/UIProcess/API/C/WKPage.h:176:  Extra space between WKPageGetStatusBarIsVisibleCallback and statusBarIsVisible  [whitespace/declaration] [3]
WebKit2/UIProcess/API/C/WKPage.h:177:  Extra space between WKPageSetStatusBarIsVisibleCallback and setStatusBarIsVisible  [whitespace/declaration] [3]
WebKit2/UIProcess/API/C/WKPage.h:178:  Extra space between WKPageGetIsResizableCallback and isResizable  [whitespace/declaration] [3]
WebKit2/UIProcess/API/C/WKPage.h:179:  Extra space between WKPageSetIsResizableCallback and setIsResizable  [whitespace/declaration] [3]
Total errors found: 8 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Early Warning System Bot 2010-10-27 20:22:03 PDT 
Attachment 72133 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4838052
Comment 4 John Sullivan 2010-10-28 06:33:32 PDT 
Comment on attachment 72133 [details]
Patch

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
Comment 5 Sam Weinig 2010-10-28 11:07:02 PDT 
Created attachment 72210 [details]
Updated.

This fixes the tools to match the names in WebKit2, but does not change ChromeClient, as that is WebCore internal.
Comment 6 WebKit Review Bot 2010-10-28 11:09:40 PDT 
Attachment 72210 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/UIProcess/API/C/WKPage.h:172:  Extra space between WKPageGetToolbarsAreVisibleCallback and toolbarsAreVisible  [whitespace/declaration] [3]
WebKit2/UIProcess/API/C/WKPage.h:173:  Extra space between WKPageSetToolbarsAreVisibleCallback and setToolbarsAreVisible  [whitespace/declaration] [3]
WebKit2/UIProcess/API/C/WKPage.h:174:  Extra space between WKPageGetMenuBarIsVisibleCallback and menuBarIsVisible  [whitespace/declaration] [3]
WebKit2/UIProcess/API/C/WKPage.h:175:  Extra space between WKPageSetMenuBarIsVisibleCallback and setMenuBarIsVisible  [whitespace/declaration] [3]
WebKit2/UIProcess/API/C/WKPage.h:176:  Extra space between WKPageGetStatusBarIsVisibleCallback and statusBarIsVisible  [whitespace/declaration] [3]
WebKit2/UIProcess/API/C/WKPage.h:177:  Extra space between WKPageSetStatusBarIsVisibleCallback and setStatusBarIsVisible  [whitespace/declaration] [3]
WebKit2/UIProcess/API/C/WKPage.h:178:  Extra space between WKPageGetIsResizableCallback and isResizable  [whitespace/declaration] [3]
WebKit2/UIProcess/API/C/WKPage.h:179:  Extra space between WKPageSetIsResizableCallback and setIsResizable  [whitespace/declaration] [3]
Total errors found: 8 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Sam Weinig 2010-10-28 11:22:04 PDT 
Landed in r70790.