Bug 226274

Summary:

[macOS] Show context menu when clicking on data detection results in image overlays

Product:

WebKit

Reporter:

Wenson Hsieh <wenson_hsieh>

Component:

Platform

Assignee:

Wenson Hsieh <wenson_hsieh>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aestes, akeerthi, bdakin, hi, megan_gardner, thorton, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

226267, 226292

Bug Blocks:

Attachments:

Description	Flags
Patch	none
Rebase on trunk	ews-feeder: commit-queue-
Fix non-internal macOS build	ews-feeder: commit-queue-
Fix non-internal macOS build (2)	ews-feeder: commit-queue-
Fix non-internal build (3)	none
For EWS	ews-feeder: commit-queue-
Patch for landing	none

Wenson Hsieh

Reported 2021-05-26 08:36:42 PDT

rdar://75504956

Attachments
Patch (39.83 KB, patch) 2021-05-27 08:46 PDT, Wenson Hsieh	no flags	Details Formatted Diff Diff
Rebase on trunk (40.12 KB, patch) 2021-05-27 13:26 PDT, Wenson Hsieh	ews-feeder: commit-queue-	Details Formatted Diff Diff
Fix non-internal macOS build (42.19 KB, patch) 2021-05-27 13:56 PDT, Wenson Hsieh	ews-feeder: commit-queue-	Details Formatted Diff Diff
Fix non-internal macOS build (2) (42.49 KB, patch) 2021-05-27 14:07 PDT, Wenson Hsieh	ews-feeder: commit-queue-	Details Formatted Diff Diff
Fix non-internal build (3) (42.51 KB, patch) 2021-05-27 14:29 PDT, Wenson Hsieh	no flags	Details Formatted Diff Diff
For EWS (42.70 KB, patch) 2021-05-27 15:47 PDT, Wenson Hsieh	ews-feeder: commit-queue-	Details Formatted Diff Diff
Patch for landing (42.78 KB, patch) 2021-05-27 20:15 PDT, Wenson Hsieh	no flags	Details Formatted Diff Diff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

Wenson Hsieh

Comment 1 2021-05-27 08:46:20 PDT Comment hidden (obsolete)

Created attachment 429881 [details] Patch

Wenson Hsieh

Comment 2 2021-05-27 13:26:51 PDT Comment hidden (obsolete)

Created attachment 429918 [details] Rebase on trunk

Wenson Hsieh

Comment 3 2021-05-27 13:56:21 PDT Comment hidden (obsolete)

Created attachment 429923 [details] Fix non-internal macOS build

Wenson Hsieh

Comment 4 2021-05-27 14:07:13 PDT Comment hidden (obsolete)

Created attachment 429926 [details] Fix non-internal macOS build (2)

Wenson Hsieh

Comment 5 2021-05-27 14:29:17 PDT

Created attachment 429928 [details] Fix non-internal build (3)

Aditya Keerthi

Comment 6 2021-05-27 15:12:48 PDT

Comment on attachment 429928 [details] Fix non-internal build (3) View in context: https://bugs.webkit.org/attachment.cgi?id=429928&action=review > Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:149 > +#import <pal/cocoa/RevealSoftLink.h> Nit: Move near the other PAL headers above? > Source/WebKit/UIProcess/mac/WKRevealItemPresenter.h:41 > +- (instancetype)initWithWebViewImpl:(WebKit::WebViewImpl&)webViewImpl item:(RVItem *)item frame:(CGRect)frameInView menuLocation:(CGPoint)menuLocationInView; const WebKit::WebViewImpl&? > Source/WebKit/UIProcess/mac/WKRevealItemPresenter.mm:36 > +#import <pal/cocoa/RevealSoftLink.h> Nit: Move PAL header before WTF header.

Tim Horton

Comment 7 2021-05-27 15:19:20 PDT

Comment on attachment 429928 [details] Fix non-internal build (3) View in context: https://bugs.webkit.org/attachment.cgi?id=429928&action=review >> Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:149 >> +#import <pal/cocoa/RevealSoftLink.h> > > Nit: Move near the other PAL headers above? SoftLinking headers go last, the stylebot enforces it (I think there is a non-style reason, but I don't remember what it is)

Wenson Hsieh

Comment 8 2021-05-27 15:21:36 PDT

Comment on attachment 429928 [details] Fix non-internal build (3) View in context: https://bugs.webkit.org/attachment.cgi?id=429928&action=review Thanks for the reviews! >>> Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:149 >>> +#import <pal/cocoa/RevealSoftLink.h> >> >> Nit: Move near the other PAL headers above? > > SoftLinking headers go last, the stylebot enforces it (I think there is a non-style reason, but I don't remember what it is) Done! >> Source/WebKit/UIProcess/mac/WKRevealItemPresenter.h:41 >> +- (instancetype)initWithWebViewImpl:(WebKit::WebViewImpl&)webViewImpl item:(RVItem *)item frame:(CGRect)frameInView menuLocation:(CGPoint)menuLocationInView; > > const WebKit::WebViewImpl&? 👍🏻 Made this `const` >> Source/WebKit/UIProcess/mac/WKRevealItemPresenter.mm:36 >> +#import <pal/cocoa/RevealSoftLink.h> > > Nit: Move PAL header before WTF header. So doing this results in this style failure: ``` % check-webkit-style ERROR: Source/WebKit/UIProcess/mac/WKRevealItemPresenter.mm:35: *SoftLink.h header should be included after all other headers. [build/include_order] [4] ERROR: Source/WebKit/UIProcess/mac/WKRevealItemPresenter.mm:36: *SoftLink.h header should be included after all other headers. [build/include_order] [4] ``` …which prompted me to move the header down below the others. I'm not exactly sure why WebViewImpl.mm doesn't yield a similar error, though 🤔. I think I'll leave this one here.

Wenson Hsieh

Comment 9 2021-05-27 15:47:20 PDT

Created attachment 429944 [details] For EWS

Wenson Hsieh

Comment 10 2021-05-27 20:15:26 PDT

Created attachment 429976 [details] Patch for landing

EWS

Comment 11 2021-05-27 20:34:52 PDT

Patch 429944 does not build

EWS

Comment 12 2021-05-27 22:10:57 PDT

Committed r278190 (238233@main): <https://commits.webkit.org/238233@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 429976 [details].

Note You need to log in before you can comment on or make changes to this bug.