| Summary: | [macOS] [WebKitLegacy] Non-actionable "Look Up" action appears when right clicking images | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Wenson Hsieh <wenson_hsieh> | ||||||
| Component: | Platform | Assignee: | 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
Wenson Hsieh
2021-06-21 15:45:31 PDT
Created attachment 431927 [details]
Patch
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")); ``` 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")); > ``` 👍🏻 Created attachment 431932 [details]
For EWS
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]. |