Bug 238473 – HTMLAttachmentElement.getAttachmentIdentifier() should propagate attachment data to the client

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Wenson Hsieh]

..in the case where the attachment element is newly created.

rdar://90938796

[Wenson Hsieh]

Created attachment 455966 [details]
Patch

[Devin Rousso]

Comment on attachment 455966 [details]
Patch

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

r=me

> Source/WebCore/editing/Editor.cpp:4233
> +        auto& [contentType, preferredFileName, data] = *attachmentInfo;
> +        client->registerAttachmentIdentifier(identifier, contentType, preferredFileName, WTFMove(data));

NIT: Should these all also be `WTFMove` since that's what's done in the IIFE above?

> Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:1736
> +    NSString *asyncScript = @"return await new Promise(resolve => {"

NIT: Is the `await` actually needed?

[Wenson Hsieh]

(In reply to Devin Rousso from comment #2)
> Comment on attachment 455966 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=455966&action=review
> 
> r=me
> 
> > Source/WebCore/editing/Editor.cpp:4233
> > +        auto& [contentType, preferredFileName, data] = *attachmentInfo;
> > +        client->registerAttachmentIdentifier(identifier, contentType, preferredFileName, WTFMove(data));
> 
> NIT: Should these all also be `WTFMove` since that's what's done in the IIFE
> above?

I'm not sure. `registerAttachmentIdentifier` takes `const String&` anyways, so I just destructured `attachmentInfo` by reference and passed the results to `registerAttachmentIdentifier`. I guess there's no harm in using WTFMove here.

> 
> > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:1736
> > +    NSString *asyncScript = @"return await new Promise(resolve => {"
> 
> NIT: Is the `await` actually needed?

Good point. It looks like `-callAsyncJavaScript:` automatically awaits in the case where the script returns a Promise.

[Wenson Hsieh]

Created attachment 455972 [details]
For EWS

[EWS]

Committed r292032 (248975@main): <https://commits.webkit.org/248975@main>

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