WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
130061
Convert MiniBrowser to use WKWebView API
https://bugs.webkit.org/show_bug.cgi?id=130061
Summary
Convert MiniBrowser to use WKWebView API
Simon Fraser (smfr)
Reported
2014-03-10 18:59:05 PDT
Convert MiniBrowser to use WKWebView API
Attachments
Patch
(37.00 KB, patch)
2014-03-10 19:01 PDT
,
Simon Fraser (smfr)
no flags
Details
Formatted Diff
Diff
Patch
(35.24 KB, patch)
2014-03-12 11:18 PDT
,
Simon Fraser (smfr)
andersca
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1
2014-03-10 19:01:42 PDT
Created
attachment 226365
[details]
Patch
Simon Fraser (smfr)
Comment 2
2014-03-10 19:02:12 PDT
I know there is some commented-out code in the patch.
Anders Carlsson
Comment 3
2014-03-10 20:44:41 PDT
Comment on
attachment 226365
[details]
Patch You still need to use the WK_API_ENABLED guards otherwise you'll break the 32-bit MiniBrowser build.
Simon Fraser (smfr)
Comment 4
2014-03-12 11:18:45 PDT
Created
attachment 226537
[details]
Patch
Anders Carlsson
Comment 5
2014-03-13 12:14:05 PDT
Comment on
attachment 226537
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=226537&action=review
> Tools/MiniBrowser/mac/WK2BrowserWindowController.m:-71 > - [_webView.browsingContextController removeObserver:self forKeyPath:@"title" context:keyValueObservingContext]; > - [_webView.browsingContextController removeObserver:self forKeyPath:@"activeURL" context:keyValueObservingContext];
You still need to remove these observers from the web view.
> Tools/MiniBrowser/mac/WK2BrowserWindowController.m:-136 > - [_webView.browsingContextController reload];
Pretty sure we have SPI to call reload.
> Tools/MiniBrowser/mac/WK2BrowserWindowController.m:179 > - return _webView && [_webView.browsingContextController canGoBack]; > + return _webView && [_webView canGoBack]; > > if (action == @selector(goForward:)) > - return _webView && [_webView.browsingContextController canGoForward]; > + return _webView && [_webView canGoForward];
No need to null check _webView here.
> Tools/MiniBrowser/mac/WK2BrowserWindowController.m:202 > - (void)applicationTerminating > { > - // FIXME: Why are we bothering to close the page? This doesn't even prevent LEAK output. > - WKPageClose(_webView.pageRef); > }
Just remove this.
> Tools/MiniBrowser/mac/WK2BrowserWindowController.m:280 > + [alert beginSheetModalForWindow:self.window completionHandler:^void (NSModalResponse response) {
No need for void here.
> Tools/MiniBrowser/mac/WK2BrowserWindowController.m:296 > + [alert beginSheetModalForWindow:self.window completionHandler:^void (NSModalResponse response) {
No need for void here.
> Tools/MiniBrowser/mac/WK2BrowserWindowController.m:297 > + completionHandler(response == NSModalResponseStop);
Is stop really the right response here?
> Tools/MiniBrowser/mac/WK2BrowserWindowController.m:320 > + // FIXME: need to send OK/Cancel back. > + completionHandler([input stringValue]);
Passing null implies cancel.
Simon Fraser (smfr)
Comment 6
2014-03-17 13:10:53 PDT
https://trac.webkit.org/r165749
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug