Bug 155395 – [iOS] WKPreviewAction conforms to NSCopying but doesn’t override -copyWithZone:

• Component: New Bugs
• Status: RESOLVED FIXED

_WKPreviewAction (which should be renamed WKPreviewAction) is a subclass of UIPreviewAction, and therefore conforms to NSCopying. It needs to override -copyWithZone: so that when it’s copied, the the copy has the same identifier.

[Radar WebKit Bug Importer]

<rdar://problem/25150528>

[Beth Dakin]

Created attachment 274011 [details]
Patch

Comment on attachment 274011 [details]
Patch

This makes a copy that doesn’t have the original’s title, style and handler!

[Beth Dakin]

Created attachment 274016 [details]
Patch

Comment on attachment 274016 [details]
Patch

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

> Source/WebKit2/UIProcess/API/Cocoa/WKPreviewActionItem.mm:43
> +    WKPreviewAction *action = [self copyWithZone:zone];

This looks infinitely recursive.

[Beth Dakin]

(In reply to comment #5)
> Comment on attachment 274016 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=274016&action=review
> 
> > Source/WebKit2/UIProcess/API/Cocoa/WKPreviewActionItem.mm:43
> > +    WKPreviewAction *action = [self copyWithZone:zone];
> 
> This looks infinitely recursive.

Eep! It does.

[Beth Dakin]

Created attachment 274021 [details]
Patch

[Beth Dakin]

Created attachment 274024 [details]
Patch

[Sam Weinig]

Comment on attachment 274024 [details]
Patch

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

You should remove the explicit conformance to NSCopying of WKPreviewAction since UIPreviewAction already conforms to it.

> Source/WebKit2/UIProcess/API/Cocoa/WKPreviewActionItem.mm:45
> +    WKPreviewAction *action = [super copyWithZone:zone];
> +    action->_identifier = self.identifier;
> +    return action;

Since WKPreviewAction is immutable, this can be implemented as:

    return [self retain];

[Sam Weinig]

(In reply to comment #9)
> Comment on attachment 274024 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=274024&action=review
> 
> You should remove the explicit conformance to NSCopying of WKPreviewAction
> since UIPreviewAction already conforms to it.
> 
> > Source/WebKit2/UIProcess/API/Cocoa/WKPreviewActionItem.mm:45
> > +    WKPreviewAction *action = [super copyWithZone:zone];
> > +    action->_identifier = self.identifier;
> > +    return action;
> 
> Since WKPreviewAction is immutable, this can be implemented as:
> 
>     return [self retain];

Actually, it looks like it isn't completely immutable, so you should do your thing.

[Beth Dakin]

Thanks Sam and Dan! http://trac.webkit.org/changeset/198166