Bug 236415 - [iOS] Add support for a "markup image" item in the callout bar when selecting a single image
Summary: [iOS] Add support for a "markup image" item in the callout bar when selecting...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on: 236403 236406
Blocks:
  Show dependency tree / graph
 
Reported: 2022-02-09 16:28 PST by Wenson Hsieh
Modified: 2022-02-11 09:03 PST (History)
7 users (show)

See Also:

Attachments
Depends on #236406 (32.95 KB, patch)
2022-02-09 18:35 PST, Wenson Hsieh 		no flags Details | Formatted Diff | Diff
Depends on #236406 (34.87 KB, patch)
2022-02-10 10:12 PST, Wenson Hsieh 		no flags Details | Formatted Diff | Diff
Rebase on trunk (35.00 KB, patch)
2022-02-10 11:37 PST, Wenson Hsieh 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Fix iOS 15 build (34.98 KB, patch)
2022-02-10 15:01 PST, Wenson Hsieh 		no flags Details | Formatted Diff | Diff
Actually fix iOS 15 build (34.86 KB, patch)
2022-02-10 16:44 PST, Wenson Hsieh 		akeerthi: review+
Details | Formatted Diff | Diff
Patch for landing (34.82 KB, patch)
2022-02-11 08:06 PST, Wenson Hsieh 		no flags Details | Formatted Diff | Diff
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2022-02-09 16:28:22 PST 
rdar://88714333
Comment 1 Wenson Hsieh 2022-02-09 18:35:49 PST Comment hidden (obsolete)
Comment 2 Wenson Hsieh 2022-02-10 10:12:15 PST Comment hidden (obsolete)
Comment 3 Wenson Hsieh 2022-02-10 11:37:15 PST Comment hidden (obsolete)
Comment 4 Wenson Hsieh 2022-02-10 15:01:45 PST Comment hidden (obsolete)
Comment 5 Wenson Hsieh 2022-02-10 16:44:23 PST 
Created attachment 451619 [details]
Actually fix iOS 15 build
Comment 6 Aditya Keerthi 2022-02-10 20:02:03 PST 
Comment on attachment 451619 [details]
Actually fix iOS 15 build

View in context: https://bugs.webkit.org/attachment.cgi?id=451619&action=review

> Source/WebKit/ChangeLog:100
> +        well, which does use custo menu items.

"custo"

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.h:248
> +struct ImageAnalysisMarkupResults {

Consider `ImageAnalysisMarkupData` or `ImageAnalysisMarkupInformation`, since this really represents a single "result".

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.h:250
> +    RetainPtr<CGImageRef> result;

Consider renaming to `image`. That name is used below when unpacking this struct.

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:376
> +static RetainPtr<NSData> transcode(CGImageRef image, CFStringRef typeIdentifier)

This helper can probably be reused in `provideDataForItem:` further down in this file.
Comment 7 Wenson Hsieh 2022-02-11 08:00:42 PST 
Comment on attachment 451619 [details]
Actually fix iOS 15 build

View in context: https://bugs.webkit.org/attachment.cgi?id=451619&action=review

Thanks for the review!

>> Source/WebKit/ChangeLog:100
>> +        well, which does use custo menu items.
> 
> "custo"

Fixed!

>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.h:248
>> +struct ImageAnalysisMarkupResults {
> 
> Consider `ImageAnalysisMarkupData` or `ImageAnalysisMarkupInformation`, since this really represents a single "result".

Makes sense! Renamed to `ImageAnalysisMarkupData` (and renamed the corresponding ivar to `_imageAnalysisMarkupData`).

>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.h:250
>> +    RetainPtr<CGImageRef> result;
> 
> Consider renaming to `image`. That name is used below when unpacking this struct.

👍🏻 Renamed to `image`.

>> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:376
>> +static RetainPtr<NSData> transcode(CGImageRef image, CFStringRef typeIdentifier)
> 
> This helper can probably be reused in `provideDataForItem:` further down in this file.

Hm…I think this patch already adjusts `-provideDataForItem:` to use this new helper function?
Comment 8 Wenson Hsieh 2022-02-11 08:06:59 PST 
Created attachment 451699 [details]
Patch for landing
Comment 9 Aditya Keerthi 2022-02-11 08:44:20 PST 
(In reply to Wenson Hsieh from comment #7)
> Comment on attachment 451619 [details]
> Actually fix iOS 15 build
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=451619&action=review
> 
> Thanks for the review!
> 
> >> Source/WebKit/ChangeLog:100
> >> +        well, which does use custo menu items.
> > 
> > "custo"
> 
> Fixed!
> 
> >> Source/WebKit/UIProcess/ios/WKContentViewInteraction.h:248
> >> +struct ImageAnalysisMarkupResults {
> > 
> > Consider `ImageAnalysisMarkupData` or `ImageAnalysisMarkupInformation`, since this really represents a single "result".
> 
> Makes sense! Renamed to `ImageAnalysisMarkupData` (and renamed the
> corresponding ivar to `_imageAnalysisMarkupData`).
> 
> >> Source/WebKit/UIProcess/ios/WKContentViewInteraction.h:250
> >> +    RetainPtr<CGImageRef> result;
> > 
> > Consider renaming to `image`. That name is used below when unpacking this struct.
> 
> 👍🏻 Renamed to `image`.
> 
> >> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:376
> >> +static RetainPtr<NSData> transcode(CGImageRef image, CFStringRef typeIdentifier)
> > 
> > This helper can probably be reused in `provideDataForItem:` further down in this file.
> 
> Hm…I think this patch already adjusts `-provideDataForItem:` to use this new
> helper function?

👍
Comment 10 EWS 2022-02-11 09:03:08 PST 
Committed r289623 (247134@main): <https://commits.webkit.org/247134@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 451699 [details].