Bug 48496 - Add WebKit2 API for window feature getter/setters
Summary: Add WebKit2 API for window feature getter/setters
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-27 20:10 PDT by Sam Weinig
Modified: 2010-10-28 11:22 PDT (History)
2 users (show)

See Also:


Attachments
Patch (18.83 KB, patch)
2010-10-27 20:12 PDT, Sam Weinig
sullivan: review-
Details | Formatted Diff | Diff
Updated. (19.64 KB, patch)
2010-10-28 11:07 PDT, Sam Weinig
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.