[webkit-reviews] review granted: [Bug 179785] [Cocoa] First pass at implementing alternative presentation button element : [Attachment 327690] Patch and layout tests

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has granted Daniel Bates
<dbates at webkit.org>'s request for review:
Bug 179785: [Cocoa] First pass at implementing alternative presentation button
element
https://bugs.webkit.org/show_bug.cgi?id=179785

Attachment 327690: Patch and layout tests

https://bugs.webkit.org/attachment.cgi?id=327690&action=review




--- Comment #22 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 327690
  --> https://bugs.webkit.org/attachment.cgi?id=327690
Patch and layout tests

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

r=me

> Source/WebCore/editing/Editor.cpp:3841
> +	   // FIXME: This substitution is only safe to do if and only if
firstElement can support a user-agent

Can you please provide a Bugzilla/Radar for this follow-up work?

> Source/WebCore/html/HTMLInputElement.h:151
> +    WEBCORE_EXPORT HTMLElement* alternativePresentationButtonElement()
const;

We need to make sure this doesn't get stored anywhere. I know this is the first
part of this set of patches, so we don't have to address it here, but we need a
mechanism where we can avoid this pointer being stored someplace.

Could this be an Optional reference?

> Source/WebCore/html/shadow/cocoa/AlternativePresentationButtonElement.cpp:100
> +    iconContainer->appendChild(icon);

This could probably be an WTFMove(icon). But not necessary to make this change.

> Source/WebCore/html/shadow/cocoa/AlternativePresentationButtonElement.cpp:110
> +    textContainer->appendChild(text);

Ditto WTFMove(text)

> Source/WebCore/html/shadow/cocoa/AlternativePresentationButtonElement.cpp:115
> +    textContainer->appendChild(subtext);

Ditto WTFMove(subtext).