Bug 224445 – [macOS] Refactor logic for presenting the shared QLPreviewPanel when revealing an image

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Wenson Hsieh]

.

[Radar WebKit Bug Importer]

<rdar://problem/76552762>

[Wenson Hsieh]

Created attachment 425781 [details]
Patch

[Andy Estes]

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?

[Devin Rousso]

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

[Wenson Hsieh]

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.

[Wenson Hsieh]

Created attachment 425912 [details]
Patch for landing

[EWS]

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].