RESOLVED FIXED 47386
Need implementation of ChromeClient windowRect related functions.
https://bugs.webkit.org/show_bug.cgi?id=47386
Summary Need implementation of ChromeClient windowRect related functions.
Sam Weinig
Reported 2010-10-07 17:43:12 PDT
Need implementation of ChromeClient windowRect related functions.
Attachments
Patch (32.54 KB, patch)
2010-10-08 21:05 PDT, Sam Weinig
no flags
Updated. (35.16 KB, patch)
2010-10-08 21:29 PDT, Sam Weinig
no flags
Patch (35.19 KB, patch)
2010-10-08 21:46 PDT, Sam Weinig
no flags
Patch (35.73 KB, patch)
2010-10-09 13:18 PDT, Sam Weinig
mitz: review+
Sam Weinig
Comment 1 2010-10-08 21:05:55 PDT
WebKit Review Bot
Comment 2 2010-10-08 21:13:34 PDT
Attachment 70336 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit2/UIProcess/WebUIClient.cpp:32: Alphabetical sorting problem. [build/include_order] [4] WebKit2/Shared/API/c/WKGeometry.h:49: Tab found; better to use spaces [whitespace/tab] [1] WebKit2/Shared/API/c/WKGeometry.h:49: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit2/UIProcess/API/C/WKPage.h:158: Extra space between WKPageGetWindowRectCallback and getWindowRect [whitespace/declaration] [3] WebKit2/UIProcess/API/C/WKPage.h:159: Extra space between WKPageSetWindowRectCallback and setWindowRect [whitespace/declaration] [3] WebKit2/UIProcess/win/WebView.cpp:37: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 6 in 29 files If any of these errors are false positives, please file a bug against check-webkit-style.
Early Warning System Bot
Comment 3 2010-10-08 21:15:00 PDT
Sam Weinig
Comment 4 2010-10-08 21:29:34 PDT
Created attachment 70338 [details] Updated.
WebKit Review Bot
Comment 5 2010-10-08 21:31:22 PDT
Attachment 70338 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit2/UIProcess/WebUIClient.cpp:32: Alphabetical sorting problem. [build/include_order] [4] WebKit2/Shared/API/c/WKGeometry.h:49: Tab found; better to use spaces [whitespace/tab] [1] WebKit2/Shared/API/c/WKGeometry.h:49: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit2/UIProcess/API/C/WKPage.h:158: Extra space between WKPageGetWindowRectCallback and getWindowRect [whitespace/declaration] [3] WebKit2/UIProcess/API/C/WKPage.h:159: Extra space between WKPageSetWindowRectCallback and setWindowRect [whitespace/declaration] [3] WebKitTools/WebKitTestRunner/win/PlatformWebViewWin.cpp:1: Missing spaces around / [whitespace/operators] [3] WebKit2/UIProcess/win/WebView.cpp:37: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 7 in 32 files If any of these errors are false positives, please file a bug against check-webkit-style.
Early Warning System Bot
Comment 6 2010-10-08 21:38:13 PDT
Sam Weinig
Comment 7 2010-10-08 21:46:58 PDT
WebKit Review Bot
Comment 8 2010-10-08 21:49:19 PDT
Attachment 70340 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit2/Shared/API/c/WKGeometry.h:49: Tab found; better to use spaces [whitespace/tab] [1] WebKit2/Shared/API/c/WKGeometry.h:49: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit2/UIProcess/API/C/WKPage.h:158: Extra space between WKPageGetWindowRectCallback and getWindowRect [whitespace/declaration] [3] WebKit2/UIProcess/API/C/WKPage.h:159: Extra space between WKPageSetWindowRectCallback and setWindowRect [whitespace/declaration] [3] WebKitTools/WebKitTestRunner/win/PlatformWebViewWin.cpp:1: Missing spaces around / [whitespace/operators] [3] Total errors found: 5 in 32 files If any of these errors are false positives, please file a bug against check-webkit-style.
mitz
Comment 9 2010-10-09 12:54:50 PDT
Comment on attachment 70336 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=70336&action=review I’d prefer “window frame” to “window rect”. The latter doesn’t mean much. > WebKit2/Shared/API/c/WKGeometry.h:49 > + WKSize size; Not enough indentation > WebKit2/UIProcess/PageClient.h:57 > + virtual WebCore::FloatRect transformRectToDeviceSpace(const WebCore::FloatRect&) = 0; > + virtual WebCore::FloatRect transformRectToUserSpace(const WebCore::FloatRect&) = 0; Is “rect” necessary in these methods’ names? > WebKit2/UIProcess/WebPageProxy.cpp:54 > +#include <WebCore/FloatRect.h> > > #include "WKContextPrivate.h" Not sure why the #includes are separated like this. > WebKit2/UIProcess/WebPageProxy.h:283 > + void windowRect(WebCore::FloatRect&); getWindowRect would be a better name. > WebKit2/UIProcess/WebUIClient.cpp:32 > #include <WebCore/IntSize.h> > +#include <WebCore/FloatRect.h> F < I > WebKit2/UIProcess/win/WebView.cpp:37 > #include <WebCore/IntRect.h> > +#include <WebCore/FloatRect.h> F < I
Sam Weinig
Comment 10 2010-10-09 13:18:45 PDT
WebKit Review Bot
Comment 11 2010-10-09 13:20:21 PDT
Attachment 70367 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit2/UIProcess/API/C/WKPage.h:158: Extra space between WKPageGetWindowFrameCallback and getWindowFrame [whitespace/declaration] [3] WebKit2/UIProcess/API/C/WKPage.h:159: Extra space between WKPageSetWindowFrameCallback and setWindowFrame [whitespace/declaration] [3] Total errors found: 2 in 32 files If any of these errors are false positives, please file a bug against check-webkit-style.
Early Warning System Bot
Comment 12 2010-10-09 13:30:06 PDT
Sam Weinig
Comment 13 2010-10-09 13:32:55 PDT
Landed in 69457.
Simon Fraser (smfr)
Comment 14 2010-10-10 18:00:12 PDT
Comment on attachment 70367 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=70367&action=review > WebKit2/UIProcess/PageClient.h:57 > + virtual WebCore::FloatRect transformToDeviceSpace(const WebCore::FloatRect&) = 0; > + virtual WebCore::FloatRect transformToUserSpace(const WebCore::FloatRect&) = 0; I'd prefer these use the term 'convert', rather than 'transform', since we use that elsewhere for similar things.
Note You need to log in before you can comment on or make changes to this bug.