Bug 227240

Summary: [macOS] [WebKitLegacy] Non-actionable "Look Up" action appears when right clicking images
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: PlatformAssignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, bdakin, cdumez, ews-watchlist, hi, japhet, megan_gardner, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
For EWS none

Wenson Hsieh
Reported 2021-06-21 15:45:31 PDT
.
Attachments
Patch (10.46 KB, patch)
2021-06-21 16:27 PDT, Wenson Hsieh
no flags
For EWS (10.36 KB, patch)
2021-06-21 17:04 PDT, Wenson Hsieh
no flags
Wenson Hsieh
Comment 1 2021-06-21 16:27:23 PDT
Devin Rousso
Comment 2 2021-06-21 16:39:37 PDT
Comment on attachment 431927 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=431927&action=review r=me > Source/WebCore/loader/EmptyClients.cpp:135 > + bool supportsLookUpInImages() final { return true; } Is this `true` because it's used in tests? Or are we assuming it's supported everywhere but WK1? > LayoutTests/fast/events/contextmenu-lookup-action-for-image.html:15 > + foundLookUpItem = false; > + for (let item of eventSender.contextClick()) { > + let title = item.title || item; > + if (title.includes?.("Look Up")) > + foundLookUpItem = true; > + } FYI you could rework this to be on one line :) ``` let foundLookUpItem = eventSender.contextClick().some((item) => (item.title || item).includes?.("Look Up")); ```
Wenson Hsieh
Comment 3 2021-06-21 17:01:20 PDT
Comment on attachment 431927 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=431927&action=review Thanks for the review! >> Source/WebCore/loader/EmptyClients.cpp:135 >> + bool supportsLookUpInImages() final { return true; } > > Is this `true` because it's used in tests? Or are we assuming it's supported everywhere but WK1? 🤦🏻‍♂️ Whoops! This was just a mistake. Good catch! Luckily, I don't think this value isn't surfaced anywhere — I'll change it to be `false`. >> LayoutTests/fast/events/contextmenu-lookup-action-for-image.html:15 >> + } > > FYI you could rework this to be on one line :) > ``` > let foundLookUpItem = eventSender.contextClick().some((item) => (item.title || item).includes?.("Look Up")); > ``` 👍🏻
Wenson Hsieh
Comment 4 2021-06-21 17:04:22 PDT
EWS
Comment 5 2021-06-21 18:22:28 PDT
Committed r279098 (239015@main): <https://commits.webkit.org/239015@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 431932 [details].
Radar WebKit Bug Importer
Comment 6 2021-06-21 18:23:16 PDT
Note You need to log in before you can comment on or make changes to this bug.