Summary: | Allow NavigationState to intercept requests and send them to SystemPreviewController | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dean Jackson <dino> | ||||
Component: | New Bugs | Assignee: | Dean Jackson <dino> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | aestes, beidson, ews-watchlist, thorton | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Dean Jackson
2018-03-09 14:32:59 PST
Created attachment 335466 [details]
Patch
Attachment 335466 [details] did not pass style-queue:
ERROR: Source/WebKit/UIProcess/SystemPreviewController.h:33: _WKPreviewControllerDataSource is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebKit/UIProcess/SystemPreviewController.h:34: _WKPreviewControllerDelegate is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
Total errors found: 2 in 5 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 335466 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=335466&action=review > Source/WebKit/UIProcess/Cocoa/NavigationState.mm:658 > + if (page.systemPreviewController()->canPreview(navigationResponse->response().mimeType())) > + page.systemPreviewController()->showPreview(navigationResponse->response().url()); It seems a bit weird to take over if the client explicitly wants to download, and also weird to then go ahead with the download? Comment on attachment 335466 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=335466&action=review >> Source/WebKit/UIProcess/Cocoa/NavigationState.mm:658 >> + page.systemPreviewController()->showPreview(navigationResponse->response().url()); > > It seems a bit weird to take over if the client explicitly wants to download, and also weird to then go ahead with the download? I agree. But we can't nicely cancel the download at this point. Ultimately, we'll have a separate download client or logic that doesn't navigate. Comment on attachment 335466 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=335466&action=review >>> Source/WebKit/UIProcess/Cocoa/NavigationState.mm:658 >>> + page.systemPreviewController()->showPreview(navigationResponse->response().url()); >> >> It seems a bit weird to take over if the client explicitly wants to download, and also weird to then go ahead with the download? > > I agree. But we can't nicely cancel the download at this point. Ultimately, we'll have a separate download client or logic that doesn't navigate. Or a separate policy type? Comment on attachment 335466 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=335466&action=review >>>> Source/WebKit/UIProcess/Cocoa/NavigationState.mm:658 >>>> + page.systemPreviewController()->showPreview(navigationResponse->response().url()); >>> >>> It seems a bit weird to take over if the client explicitly wants to download, and also weird to then go ahead with the download? >> >> I agree. But we can't nicely cancel the download at this point. Ultimately, we'll have a separate download client or logic that doesn't navigate. > > Or a separate policy type? Actually, it sort of works if I ignore the download! I originally considered a new policy type, but this one is actually set from Safari. I'll have to do something else for WKWebView clients. Committed r229488: <https://trac.webkit.org/changeset/229488> |