Bug 237810

Summary: [Mail compose] Allow copied resources with remote (HTTP/HTTPS) URLs to be pasted as attachments
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: HTML EditingAssignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: akeerthi, ews-watchlist, hi, katherine_cheney, megan_gardner, mifenton, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
For landing none

Description Wenson Hsieh 2022-03-12 14:05:26 PST 
rdar://90119983
Comment 1 Wenson Hsieh 2022-03-12 14:36:43 PST 
Created attachment 454541 [details]
Patch
Comment 2 Devin Rousso 2022-03-14 14:15:52 PDT 
Comment on attachment 454541 [details]
Patch

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

r=me, neato :)

> Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm:209
> +    return replacementType == ReplacementMethod::Attachment || !url.protocolIsInHTTPFamily();

NIT: I feel like this would read better as a separate if.
```
if (url.protocolIsInHTTPFamily())
    return replacementType == ReplacementMethod::Attachment;

return true;
```
Comment 3 Wenson Hsieh 2022-03-14 14:17:41 PDT 
Comment on attachment 454541 [details]
Patch

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

Thanks for the review!

>> Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm:209
>> +    return replacementType == ReplacementMethod::Attachment || !url.protocolIsInHTTPFamily();
> 
> NIT: I feel like this would read better as a separate if.
> ```
> if (url.protocolIsInHTTPFamily())
>     return replacementType == ReplacementMethod::Attachment;
> 
> return true;
> ```

Sounds good — will change it to that.
Comment 4 Wenson Hsieh 2022-03-14 14:34:32 PDT 
Created attachment 454622 [details]
For landing
Comment 5 EWS 2022-03-14 18:56:23 PDT 
Committed r291261 (248413@main): <https://commits.webkit.org/248413@main>

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