Bug 237127

Summary: Refactor logic for showing "Markup Image" and Quick Note items in the callout bar
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: PlatformAssignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: akeerthi, hi, katherine_cheney, megan_gardner, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
For EWS
megan_gardner: review+
For landing none

Wenson Hsieh
Reported 2022-02-23 20:48:17 PST
.
Attachments
For EWS (34.22 KB, patch)
2022-02-23 21:12 PST, Wenson Hsieh 		megan_gardner: review+
DetailsFormatted DiffDiff
For landing (34.45 KB, patch)
2022-02-24 09:33 PST, Wenson Hsieh 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2022-02-23 20:48:33 PST
<rdar://problem/89396617>
Wenson Hsieh
Comment 2 2022-02-23 21:12:26 PST
Created attachment 453068 [details] For EWS
Megan Gardner
Comment 3 2022-02-24 08:11:58 PST
Comment on attachment 453068 [details] For EWS View in context: https://bugs.webkit.org/attachment.cgi?id=453068&action=review > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKContentViewEditingActions.mm:96 > + EXPECT_FALSE([menuBuilder containsActionWithTitle:WebCore::contextMenuItemTagAddHighlightToNewQuickNote()]); You should still test for both App Highlight menu items. If things change around the logic for which item to show when, it's unlikely that this test would be updated as a part of that. So, best to just do it now. > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKContentViewEditingActions.mm:104 > + EXPECT_TRUE([menuBuilder containsActionWithTitle:WebCore::contextMenuItemTagAddHighlightToNewQuickNote()]); ditto.
Wenson Hsieh
Comment 4 2022-02-24 09:09:14 PST
Comment on attachment 453068 [details] For EWS View in context: https://bugs.webkit.org/attachment.cgi?id=453068&action=review Thanks for the review! >> Tools/TestWebKitAPI/Tests/WebKitCocoa/WKContentViewEditingActions.mm:96 >> + EXPECT_FALSE([menuBuilder containsActionWithTitle:WebCore::contextMenuItemTagAddHighlightToNewQuickNote()]); > > You should still test for both App Highlight menu items. If things change around the logic for which item to show when, it's unlikely that this test would be updated as a part of that. So, best to just do it now. Sounds good — I'll add checks to verify that the "Add to existing Quick Note" items are not also present in either case.
Wenson Hsieh
Comment 5 2022-02-24 09:33:48 PST
Created attachment 453109 [details] For landing
EWS
Comment 6 2022-02-24 12:15:00 PST
Committed r290450 (247752@main): <https://commits.webkit.org/247752@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 453109 [details].
Note You need to log in before you can comment on or make changes to this bug.