155395 2016-03-12 12:04:50 -0800 [iOS] WKPreviewAction conforms to NSCopying but doesn’t override -copyWithZone: 2016-03-14 15:58:51 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Local Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=155383 InRadar P2 Normal --- 1 mitz webkit-unassigned andersca bdakin mitz sam thorton webkit-bug-importer oldest_to_newest 1174060 0 mitz 2016-03-12 12:04:50 -0800 _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. 1174652 1 webkit-bug-importer 2016-03-14 13:30:15 -0700 <rdar://problem/25150528> 1174655 2 274011 bdakin 2016-03-14 13:31:43 -0700 Created attachment 274011 Patch 1174658 3 274011 mitz 2016-03-14 13:33:58 -0700 Comment on attachment 274011 Patch This makes a copy that doesn’t have the original’s title, style and handler! 1174668 4 274016 bdakin 2016-03-14 13:38:16 -0700 Created attachment 274016 Patch 1174670 5 274016 mitz 2016-03-14 13:40:31 -0700 Comment on attachment 274016 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. 1174675 6 bdakin 2016-03-14 13:42:40 -0700 (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. 1174679 7 274021 bdakin 2016-03-14 13:49:31 -0700 Created attachment 274021 Patch 1174693 8 274024 bdakin 2016-03-14 14:01:15 -0700 Created attachment 274024 Patch 1174761 9 274024 sam 2016-03-14 15:51:57 -0700 Comment on attachment 274024 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]; 1174765 10 sam 2016-03-14 15:55:24 -0700 (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. 1174769 11 bdakin 2016-03-14 15:58:51 -0700 Thanks Sam and Dan! http://trac.webkit.org/changeset/198166 274011 2016-03-14 13:31:43 -0700 2016-03-14 13:38:16 -0700 Patch for-review.txt text/plain 1314 bdakin SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDE5ODE1MikKKysrIFNvdXJjZS9XZWJLaXQyL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE0IEBACisyMDE2LTAzLTE0ICBCZXRoIERh a2luICA8YmRha2luQGFwcGxlLmNvbT4KKworICAgICAgICBbaU9TXSBXS1ByZXZpZXdBY3Rpb24g Y29uZm9ybXMgdG8gTlNDb3B5aW5nIGJ1dCBkb2VzbuKAmXQgb3ZlcnJpZGUgLQorICAgICAgICBj b3B5V2l0aFpvbmU6CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD0xNTUzOTUKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAg ICAgICAqIFVJUHJvY2Vzcy9BUEkvQ29jb2EvV0tQcmV2aWV3QWN0aW9uSXRlbS5tbToKKyAgICAg ICAgKC1bV0tQcmV2aWV3QWN0aW9uIGNvcHlXaXRoWm9uZTpdKToKKwogMjAxNi0wMy0xNCAgVGlt IEhvcnRvbiAgPHRpbW90aHlfaG9ydG9uQGFwcGxlLmNvbT4KIAogICAgICAgICBSZXZlcnQgcjE5 NDEyNSBhbmQgcjE5NDE4NjogV2UncmUgZ29pbmcgdG8gZml4IHRoaXMgYSBkaWZmZXJlbnQgd2F5 LgpJbmRleDogU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQSS9Db2NvYS9XS1ByZXZpZXdBY3Rp b25JdGVtLm1tCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkvQ29j b2EvV0tQcmV2aWV3QWN0aW9uSXRlbS5tbQkocmV2aXNpb24gMTk4MTM5KQorKysgU291cmNlL1dl YktpdDIvVUlQcm9jZXNzL0FQSS9Db2NvYS9XS1ByZXZpZXdBY3Rpb25JdGVtLm1tCSh3b3JraW5n IGNvcHkpCkBAIC0zOCw2ICszOCwxMyBAQCArIChpbnN0YW5jZXR5cGUpYWN0aW9uV2l0aElkZW50 aWZpZXI6KE5TCiAgICAgcmV0dXJuIGFjdGlvbjsKIH0KIAorLSAoaWQpY29weVdpdGhab25lOihO U1pvbmUgKil6b25lCit7CisgICAgV0tQcmV2aWV3QWN0aW9uICphY3Rpb24gPSBbKFdLUHJldmll d0FjdGlvbiAqKVtbc2VsZiBjbGFzc10gYWxsb2NXaXRoWm9uZTp6b25lXSBpbml0XTsKKyAgICBh Y3Rpb24tPl9pZGVudGlmaWVyID0gc2VsZi5pZGVudGlmaWVyOworICAgIHJldHVybiBhY3Rpb247 Cit9CisKIEBlbmQKIAogI2VuZGlmIC8vIFdLX0FQSV9FTkFCTEVEICYmIFRBUkdFVF9PU19JUEhP TkUK 274016 2016-03-14 13:38:16 -0700 2016-03-14 13:49:31 -0700 Patch for-review-2.txt text/plain 1279 bdakin SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDE5ODE1MikKKysrIFNvdXJjZS9XZWJLaXQyL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE0IEBACisyMDE2LTAzLTE0ICBCZXRoIERh a2luICA8YmRha2luQGFwcGxlLmNvbT4KKworICAgICAgICBbaU9TXSBXS1ByZXZpZXdBY3Rpb24g Y29uZm9ybXMgdG8gTlNDb3B5aW5nIGJ1dCBkb2VzbuKAmXQgb3ZlcnJpZGUgLQorICAgICAgICBj b3B5V2l0aFpvbmU6CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD0xNTUzOTUKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAg ICAgICAqIFVJUHJvY2Vzcy9BUEkvQ29jb2EvV0tQcmV2aWV3QWN0aW9uSXRlbS5tbToKKyAgICAg ICAgKC1bV0tQcmV2aWV3QWN0aW9uIGNvcHlXaXRoWm9uZTpdKToKKwogMjAxNi0wMy0xNCAgVGlt IEhvcnRvbiAgPHRpbW90aHlfaG9ydG9uQGFwcGxlLmNvbT4KIAogICAgICAgICBSZXZlcnQgcjE5 NDEyNSBhbmQgcjE5NDE4NjogV2UncmUgZ29pbmcgdG8gZml4IHRoaXMgYSBkaWZmZXJlbnQgd2F5 LgpJbmRleDogU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQSS9Db2NvYS9XS1ByZXZpZXdBY3Rp b25JdGVtLm1tCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkvQ29j b2EvV0tQcmV2aWV3QWN0aW9uSXRlbS5tbQkocmV2aXNpb24gMTk4MTM5KQorKysgU291cmNlL1dl YktpdDIvVUlQcm9jZXNzL0FQSS9Db2NvYS9XS1ByZXZpZXdBY3Rpb25JdGVtLm1tCSh3b3JraW5n IGNvcHkpCkBAIC0zOCw2ICszOCwxMyBAQCArIChpbnN0YW5jZXR5cGUpYWN0aW9uV2l0aElkZW50 aWZpZXI6KE5TCiAgICAgcmV0dXJuIGFjdGlvbjsKIH0KIAorLSAoaWQpY29weVdpdGhab25lOihO U1pvbmUgKil6b25lCit7CisgICAgV0tQcmV2aWV3QWN0aW9uICphY3Rpb24gPSBbc2VsZiBjb3B5 V2l0aFpvbmU6em9uZV07CisgICAgYWN0aW9uLT5faWRlbnRpZmllciA9IHNlbGYuaWRlbnRpZmll cjsKKyAgICByZXR1cm4gYWN0aW9uOworfQorCiBAZW5kCiAKICNlbmRpZiAvLyBXS19BUElfRU5B QkxFRCAmJiBUQVJHRVRfT1NfSVBIT05FCg== 274021 2016-03-14 13:49:31 -0700 2016-03-14 14:01:15 -0700 Patch for-review-3.txt text/plain 1290 bdakin SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDE5ODE1MikKKysrIFNvdXJjZS9XZWJLaXQyL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE0IEBACisyMDE2LTAzLTE0ICBCZXRoIERh a2luICA8YmRha2luQGFwcGxlLmNvbT4KKworICAgICAgICBbaU9TXSBXS1ByZXZpZXdBY3Rpb24g Y29uZm9ybXMgdG8gTlNDb3B5aW5nIGJ1dCBkb2VzbuKAmXQgb3ZlcnJpZGUgLQorICAgICAgICBj b3B5V2l0aFpvbmU6CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD0xNTUzOTUKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAg ICAgICAqIFVJUHJvY2Vzcy9BUEkvQ29jb2EvV0tQcmV2aWV3QWN0aW9uSXRlbS5tbToKKyAgICAg ICAgKC1bV0tQcmV2aWV3QWN0aW9uIGNvcHlXaXRoWm9uZTpdKToKKwogMjAxNi0wMy0xNCAgVGlt IEhvcnRvbiAgPHRpbW90aHlfaG9ydG9uQGFwcGxlLmNvbT4KIAogICAgICAgICBSZXZlcnQgcjE5 NDEyNSBhbmQgcjE5NDE4NjogV2UncmUgZ29pbmcgdG8gZml4IHRoaXMgYSBkaWZmZXJlbnQgd2F5 LgpJbmRleDogU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQSS9Db2NvYS9XS1ByZXZpZXdBY3Rp b25JdGVtLm1tCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkvQ29j b2EvV0tQcmV2aWV3QWN0aW9uSXRlbS5tbQkocmV2aXNpb24gMTk4MTM5KQorKysgU291cmNlL1dl YktpdDIvVUlQcm9jZXNzL0FQSS9Db2NvYS9XS1ByZXZpZXdBY3Rpb25JdGVtLm1tCSh3b3JraW5n IGNvcHkpCkBAIC0zOCw2ICszOCwxMyBAQCArIChpbnN0YW5jZXR5cGUpYWN0aW9uV2l0aElkZW50 aWZpZXI6KE5TCiAgICAgcmV0dXJuIGFjdGlvbjsKIH0KIAorLSAoaWQpY29weVdpdGhab25lOihO U1pvbmUgKil6b25lCit7CisgICAgV0tQcmV2aWV3QWN0aW9uICphY3Rpb24gPSBbVUlQcmV2aWV3 QWN0aW9uIGNvcHlXaXRoWm9uZTp6b25lXTsKKyAgICBhY3Rpb24tPl9pZGVudGlmaWVyID0gc2Vs Zi5pZGVudGlmaWVyOworICAgIHJldHVybiBhY3Rpb247Cit9CisKIEBlbmQKIAogI2VuZGlmIC8v IFdLX0FQSV9FTkFCTEVEICYmIFRBUkdFVF9PU19JUEhPTkUK 274024 2016-03-14 14:01:15 -0700 2016-03-14 15:51:57 -0700 Patch for-review-4.txt text/plain 1280 bdakin SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDE5ODE1MikKKysrIFNvdXJjZS9XZWJLaXQyL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE0IEBACisyMDE2LTAzLTE0ICBCZXRoIERh a2luICA8YmRha2luQGFwcGxlLmNvbT4KKworICAgICAgICBbaU9TXSBXS1ByZXZpZXdBY3Rpb24g Y29uZm9ybXMgdG8gTlNDb3B5aW5nIGJ1dCBkb2VzbuKAmXQgb3ZlcnJpZGUgLQorICAgICAgICBj b3B5V2l0aFpvbmU6CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD0xNTUzOTUKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAg ICAgICAqIFVJUHJvY2Vzcy9BUEkvQ29jb2EvV0tQcmV2aWV3QWN0aW9uSXRlbS5tbToKKyAgICAg ICAgKC1bV0tQcmV2aWV3QWN0aW9uIGNvcHlXaXRoWm9uZTpdKToKKwogMjAxNi0wMy0xNCAgVGlt IEhvcnRvbiAgPHRpbW90aHlfaG9ydG9uQGFwcGxlLmNvbT4KIAogICAgICAgICBSZXZlcnQgcjE5 NDEyNSBhbmQgcjE5NDE4NjogV2UncmUgZ29pbmcgdG8gZml4IHRoaXMgYSBkaWZmZXJlbnQgd2F5 LgpJbmRleDogU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL0FQSS9Db2NvYS9XS1ByZXZpZXdBY3Rp b25JdGVtLm1tCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9BUEkvQ29j b2EvV0tQcmV2aWV3QWN0aW9uSXRlbS5tbQkocmV2aXNpb24gMTk4MTM5KQorKysgU291cmNlL1dl YktpdDIvVUlQcm9jZXNzL0FQSS9Db2NvYS9XS1ByZXZpZXdBY3Rpb25JdGVtLm1tCSh3b3JraW5n IGNvcHkpCkBAIC0zOCw2ICszOCwxMyBAQCArIChpbnN0YW5jZXR5cGUpYWN0aW9uV2l0aElkZW50 aWZpZXI6KE5TCiAgICAgcmV0dXJuIGFjdGlvbjsKIH0KIAorLSAoaWQpY29weVdpdGhab25lOihO U1pvbmUgKil6b25lCit7CisgICAgV0tQcmV2aWV3QWN0aW9uICphY3Rpb24gPSBbc3VwZXIgY29w eVdpdGhab25lOnpvbmVdOworICAgIGFjdGlvbi0+X2lkZW50aWZpZXIgPSBzZWxmLmlkZW50aWZp ZXI7CisgICAgcmV0dXJuIGFjdGlvbjsKK30KKwogQGVuZAogCiAjZW5kaWYgLy8gV0tfQVBJX0VO QUJMRUQgJiYgVEFSR0VUX09TX0lQSE9ORQo=