Bug 224445

Summary: [macOS] Refactor logic for presenting the shared QLPreviewPanel when revealing an image
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: PlatformAssignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, bdakin, hi, megan_gardner, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
aestes: review+
Patch for landing none

Description Wenson Hsieh 2021-04-12 12:40:16 PDT 
.
Comment 1 Radar WebKit Bug Importer 2021-04-12 12:46:04 PDT 
<rdar://problem/76552762>
Comment 2 Wenson Hsieh 2021-04-12 13:27:25 PDT 
Created attachment 425781 [details]
Patch
Comment 3 Andy Estes 2021-04-13 12:52:34 PDT 
Comment on attachment 425781 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=425781&action=review

> Source/WebKit/UIProcess/API/mac/WKWebViewMac.mm:46
> +@class QLPreviewPanel;

Do you still need this since WebViewImpl.h also declares QLPreviewPanel?

> Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:139
> +#import "WKImageExtractionPreviewController.h"

Could this be imported unconditionally?
Comment 4 Devin Rousso 2021-04-13 13:07:03 PDT 
Comment on attachment 425781 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=425781&action=review

r=me as well :)

> Source/WebKit/ChangeLog:23
> +        To address this, we refactor this logic is that we make `WKWebView` (or `WKView`, if applicable) capable of

oops
Comment 5 Wenson Hsieh 2021-04-13 13:35:58 PDT 
Comment on attachment 425781 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=425781&action=review

Thanks for the reviews!

>> Source/WebKit/ChangeLog:23
>> +        To address this, we refactor this logic is that we make `WKWebView` (or `WKView`, if applicable) capable of
> 
> oops

😅

>> Source/WebKit/UIProcess/API/mac/WKWebViewMac.mm:46
>> +@class QLPreviewPanel;
> 
> Do you still need this since WebViewImpl.h also declares QLPreviewPanel?

Good catch! I should be able to remove this.

>> Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:139
>> +#import "WKImageExtractionPreviewController.h"
> 
> Could this be imported unconditionally?

Ah, yes, both of these headers can be imported unconditionally since the header contents are already guarded by the flag. I'll move them out of here and remove the #if.
Comment 6 Wenson Hsieh 2021-04-13 14:14:49 PDT 
Created attachment 425912 [details]
Patch for landing
Comment 7 EWS 2021-04-13 14:44:40 PDT 
Committed r275913 (236477@main): <https://commits.webkit.org/236477@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 425912 [details].