191141 2018-10-31 17:25:51 -0700 Forward original fragment identifier into System Preview 2018-10-31 18:03:41 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 dino dino webkit-bug-importer wenson_hsieh oldest_to_newest 1473961 0 dino 2018-10-31 17:25:51 -0700 Forward original fragment identifier into System Preview 1473962 1 webkit-bug-importer 2018-10-31 17:29:41 -0700 <rdar://problem/45717542> 1473963 2 353561 dino 2018-10-31 17:32:25 -0700 Created attachment 353561 Patch 1473967 3 353561 wenson_hsieh 2018-10-31 17:45:23 -0700 Comment on attachment 353561 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353561&action=review r=mews > Source/WebKit/UIProcess/Cocoa/SystemPreviewControllerCocoa.mm:108 > +- (void)finish:(WebCore::URL)url withFragmentIdentifier:(NSString*)fragmentIdentifier Nit - NSString * > Source/WebKit/UIProcess/Cocoa/SystemPreviewControllerCocoa.mm:114 > + self.completionHandler([NSURL URLWithString:[NSString stringWithFormat:@"%@#%@", (NSString*)url.string(), (NSString*)fragmentIdentifier]], nil); Nit - More NSString * Instead of passing the fragment separately, would it be cleaner to just use URL::setFragmentIdentifier on the destinationURL? 1473968 4 353561 dino 2018-10-31 17:47:13 -0700 Comment on attachment 353561 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353561&action=review >> Source/WebKit/UIProcess/Cocoa/SystemPreviewControllerCocoa.mm:114 >> + self.completionHandler([NSURL URLWithString:[NSString stringWithFormat:@"%@#%@", (NSString*)url.string(), (NSString*)fragmentIdentifier]], nil); > > Nit - More NSString * > > Instead of passing the fragment separately, would it be cleaner to just use URL::setFragmentIdentifier on the destinationURL? Yes. I forgot that I didn't have an NSURL here (which are immutable). 1473969 5 353561 dino 2018-10-31 17:49:32 -0700 Comment on attachment 353561 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353561&action=review >>> Source/WebKit/UIProcess/Cocoa/SystemPreviewControllerCocoa.mm:114 >>> + self.completionHandler([NSURL URLWithString:[NSString stringWithFormat:@"%@#%@", (NSString*)url.string(), (NSString*)fragmentIdentifier]], nil); >> >> Nit - More NSString * >> >> Instead of passing the fragment separately, would it be cleaner to just use URL::setFragmentIdentifier on the destinationURL? > > Yes. I forgot that I didn't have an NSURL here (which are immutable). Oh wait. I was right - I do get an NSURL. I then change it into a WebCore::URL, then back into an NSURL, then into another NSURL with the fragment. I can definitely clean this up. 1473977 6 dino 2018-10-31 18:03:41 -0700 Committed r237664: <https://trac.webkit.org/changeset/237664> 353561 2018-10-31 17:32:25 -0700 2018-10-31 17:45:23 -0700 Patch bug-191141-20181101113224.patch text/plain 4755 dino U3VidmVyc2lvbiBSZXZpc2lvbjogMjM3NjQzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IGY2ZGY0MTYyNGQ0OWMyMGYy NGUwNmQ5Y2ZjNGIxOTFhYTEzNmJhYzguLmU1N2RiNjU3MmJkNDcwMTY5YjZlNzRmZGQyOGM2ZGQw MTJjNzdjMzcgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjQgQEAKKzIwMTgtMTAtMzEgIERlYW4gSmFj a3NvbiAgPGRpbm9AYXBwbGUuY29tPgorCisgICAgICAgIEZvcndhcmQgb3JpZ2luYWwgZnJhZ21l bnQgaWRlbnRpZmllciBpbnRvIFN5c3RlbSBQcmV2aWV3CisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xOTExNDEKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVt LzQ1NzE3NTQyPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgIElmIHRoZSBVUkwgdXNlZCBieSB0aGUgcGFnZSBpbmNsdWRlcyBhIGZyYWdtZW50IGlkZW50 aWZpZXIsIHVzZQorICAgICAgICBpdCB3aGVuIHRlbGxpbmcgUXVpY2tMb29rIHRvIG9wZW4gYSBs b2NhbCBmaWxlLgorCisgICAgICAgICogVUlQcm9jZXNzL0NvY29hL0Rvd25sb2FkQ2xpZW50Lm1t OgorICAgICAgICAoV2ViS2l0OjpEb3dubG9hZENsaWVudDo6ZGlkRmluaXNoKTogRm9yd2FyZCBv cmlnaW5hbCBmcmFnbWVudCBpZGVudGlmaWVyLgorICAgICAgICAqIFVJUHJvY2Vzcy9Db2NvYS9T eXN0ZW1QcmV2aWV3Q29udHJvbGxlckNvY29hLm1tOgorICAgICAgICAoLVtfV0tQcmV2aWV3Q29u dHJvbGxlckRhdGFTb3VyY2UgZmluaXNoOndpdGhGcmFnbWVudElkZW50aWZpZXI6XSk6CisgICAg ICAgICAgICBJZiB0aGVyZSB3YXMgYSBmcmFnbWVudCBpZGVudGlmaWVyLCBhcHBlbmQgaXQgdG8g dGhlIGVuZCBvZiB0aGUgbG9jYWwKKyAgICAgICAgICAgIGZpbGU6Ly8gVVJMIHRoYXQgd2Ugc2Vu ZCBpbnRvIHRoZSBOU0l0ZW1Qcm92aWRlckNvbXBsZXRpb25IYW5kbGVyLgorICAgICAgICAoV2Vi S2l0OjpTeXN0ZW1QcmV2aWV3Q29udHJvbGxlcjo6ZmluaXNoKTogUmVuYW1lZC4KKyAgICAgICAg KC1bX1dLUHJldmlld0NvbnRyb2xsZXJEYXRhU291cmNlIGZpbmlzaDpdKTogUmVuYW1lZC4uCisg ICAgICAgICogVUlQcm9jZXNzL1N5c3RlbVByZXZpZXdDb250cm9sbGVyLmg6CisKIDIwMTgtMTAt MzEgIENocmlzIER1bWV6ICA8Y2R1bWV6QGFwcGxlLmNvbT4KIAogICAgICAgICBbUFNPTl0gV2hl biBwcm9jZXNzLXN3YXBwaW5nIGZvciBhIFBPU1QgcmVxdWVzdCB0aGUgSFRUUCBib2R5IGdldHMg ZHJvcHBlZApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvQ29jb2EvRG93bmxv YWRDbGllbnQubW0gYi9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9Db2NvYS9Eb3dubG9hZENsaWVu dC5tbQppbmRleCAwMDg0ZjU2MDdjODc2MDFmNjFjN2RmM2JjMTAzOTJlOTlmY2I0NmNlLi5lOGZm YWQ3NzhkYzVlYmU5ZjI2OTNlMjJmNzFiOTQ5NTA4NjEwNDIyIDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViS2l0L1VJUHJvY2Vzcy9Db2NvYS9Eb3dubG9hZENsaWVudC5tbQorKysgYi9Tb3VyY2UvV2Vi S2l0L1VJUHJvY2Vzcy9Db2NvYS9Eb3dubG9hZENsaWVudC5tbQpAQCAtMjEzLDcgKzIxMyw4IEBA IHZvaWQgRG93bmxvYWRDbGllbnQ6OmRpZEZpbmlzaChXZWJQcm9jZXNzUG9vbCYsIERvd25sb2Fk UHJveHkmIGRvd25sb2FkUHJveHkpCiAgICAgaWYgKGRvd25sb2FkUHJveHkuaXNTeXN0ZW1QcmV2 aWV3RG93bmxvYWQoKSkgewogICAgICAgICBpZiAoYXV0byogd2ViUGFnZSA9IGRvd25sb2FkUHJv eHkub3JpZ2luYXRpbmdQYWdlKCkpIHsKICAgICAgICAgICAgIE5TVVJMICpkZXN0aW5hdGlvblVS TCA9IFtOU1VSTCBmaWxlVVJMV2l0aFBhdGg6KE5TU3RyaW5nICopZG93bmxvYWRQcm94eS5kZXN0 aW5hdGlvbkZpbGVuYW1lKCldOwotICAgICAgICAgICAgd2ViUGFnZS0+c3lzdGVtUHJldmlld0Nv bnRyb2xsZXIoKS0+ZmluaXNoKFdlYkNvcmU6OlVSTChkZXN0aW5hdGlvblVSTCkpOworICAgICAg ICAgICAgU3RyaW5nIG9yaWdpbmFsRnJhZ21lbnRJZGVudGlmaWVyID0gZG93bmxvYWRQcm94eS5y ZXF1ZXN0KCkudXJsKCkuZnJhZ21lbnRJZGVudGlmaWVyKCk7CisgICAgICAgICAgICB3ZWJQYWdl LT5zeXN0ZW1QcmV2aWV3Q29udHJvbGxlcigpLT5maW5pc2goV2ViQ29yZTo6VVJMKGRlc3RpbmF0 aW9uVVJMKSwgb3JpZ2luYWxGcmFnbWVudElkZW50aWZpZXIpOwogICAgICAgICB9CiAgICAgICAg IHJlbGVhc2VBY3Rpdml0eVRva2VuSWZOZWNlc3NhcnkoZG93bmxvYWRQcm94eSk7CiAgICAgICAg IHJldHVybjsKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL0NvY29hL1N5c3Rl bVByZXZpZXdDb250cm9sbGVyQ29jb2EubW0gYi9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9Db2Nv YS9TeXN0ZW1QcmV2aWV3Q29udHJvbGxlckNvY29hLm1tCmluZGV4IGE3ZGFkNDgxNTI2MWE5OGM2 YzExZDM0NDhmM2RiMjE4MTMxYTYyZGEuLjQxNTZmYmUzODk4MGJmMjFjYjJiM2FkMjI2MTk1Mzhi ZGVhZGI4OTggMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL0NvY29hL1N5c3Rl bVByZXZpZXdDb250cm9sbGVyQ29jb2EubW0KKysrIGIvU291cmNlL1dlYktpdC9VSVByb2Nlc3Mv Q29jb2EvU3lzdGVtUHJldmlld0NvbnRyb2xsZXJDb2NvYS5tbQpAQCAtMTA1LDEwICsxMDUsMTQg QEAgU09GVF9MSU5LX0NMQVNTKFF1aWNrTG9vaywgUUxJdGVtKTsKICAgICAgICAgW19pdGVtIHNl dFByZXZpZXdJdGVtUHJvdmlkZXJQcm9ncmVzczpAKHByb2dyZXNzKV07CiB9CiAKLS0gKHZvaWQp ZmluaXNoOihXZWJDb3JlOjpVUkwpdXJsCistICh2b2lkKWZpbmlzaDooV2ViQ29yZTo6VVJMKXVy bCB3aXRoRnJhZ21lbnRJZGVudGlmaWVyOihOU1N0cmluZyopZnJhZ21lbnRJZGVudGlmaWVyCiB7 Ci0gICAgaWYgKHNlbGYuY29tcGxldGlvbkhhbmRsZXIpCi0gICAgICAgIHNlbGYuY29tcGxldGlv bkhhbmRsZXIoKE5TVVJMKil1cmwsIG5pbCk7CisgICAgaWYgKHNlbGYuY29tcGxldGlvbkhhbmRs ZXIpIHsKKyAgICAgICAgaWYgKCFmcmFnbWVudElkZW50aWZpZXIgfHwgIWZyYWdtZW50SWRlbnRp Zmllci5sZW5ndGggfHwgdXJsLmZyYWdtZW50SWRlbnRpZmllcigpLmxlbmd0aCgpKQorICAgICAg ICAgICAgc2VsZi5jb21wbGV0aW9uSGFuZGxlcigoTlNVUkwqKXVybCwgbmlsKTsKKyAgICAgICAg ZWxzZQorICAgICAgICAgICAgc2VsZi5jb21wbGV0aW9uSGFuZGxlcihbTlNVUkwgVVJMV2l0aFN0 cmluZzpbTlNTdHJpbmcgc3RyaW5nV2l0aEZvcm1hdDpAIiVAIyVAIiwgKE5TU3RyaW5nKil1cmwu c3RyaW5nKCksIChOU1N0cmluZyopZnJhZ21lbnRJZGVudGlmaWVyXV0sIG5pbCk7CisgICAgfQog fQogCiAtICh2b2lkKWZhaWxXaXRoRXJyb3I6KE5TRXJyb3IgKillcnJvcgpAQCAtMjIxLDEwICsy MjUsMTAgQEAgdm9pZCBTeXN0ZW1QcmV2aWV3Q29udHJvbGxlcjo6dXBkYXRlUHJvZ3Jlc3MoZmxv YXQgcHJvZ3Jlc3MpCiAgICAgICAgIFttX3FsUHJldmlld0NvbnRyb2xsZXJEYXRhU291cmNlIHNl dFByb2dyZXNzOnByb2dyZXNzXTsKIH0KIAotdm9pZCBTeXN0ZW1QcmV2aWV3Q29udHJvbGxlcjo6 ZmluaXNoKFdlYkNvcmU6OlVSTCB1cmwpCit2b2lkIFN5c3RlbVByZXZpZXdDb250cm9sbGVyOjpm aW5pc2goV2ViQ29yZTo6VVJMIGRlc3RpbmF0aW9uTG9jYXRpb24sIGNvbnN0IFN0cmluZyYgb3Jp Z2luYWxGcmFnbWVudElkZW50aWZpZXIpCiB7CiAgICAgaWYgKG1fcWxQcmV2aWV3Q29udHJvbGxl ckRhdGFTb3VyY2UpCi0gICAgICAgIFttX3FsUHJldmlld0NvbnRyb2xsZXJEYXRhU291cmNlIGZp bmlzaDp1cmxdOworICAgICAgICBbbV9xbFByZXZpZXdDb250cm9sbGVyRGF0YVNvdXJjZSBmaW5p c2g6ZGVzdGluYXRpb25Mb2NhdGlvbiB3aXRoRnJhZ21lbnRJZGVudGlmaWVyOm9yaWdpbmFsRnJh Z21lbnRJZGVudGlmaWVyXTsKIH0KIAogdm9pZCBTeXN0ZW1QcmV2aWV3Q29udHJvbGxlcjo6Y2Fu Y2VsKCkKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL1N5c3RlbVByZXZpZXdD b250cm9sbGVyLmggYi9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9TeXN0ZW1QcmV2aWV3Q29udHJv bGxlci5oCmluZGV4IDk0OWM4ZGY3MjFlZDllZDM1YTdkYTY4ZWM3Y2Y5MTI3OWVmOTBhMzEuLjhj NWQzMjg1MDM2ZjM0Mzg4NjgyODM0OTM5YzJiMWI5ZjkyY2ZhMzQgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJLaXQvVUlQcm9jZXNzL1N5c3RlbVByZXZpZXdDb250cm9sbGVyLmgKKysrIGIvU291cmNl L1dlYktpdC9VSVByb2Nlc3MvU3lzdGVtUHJldmlld0NvbnRyb2xsZXIuaApAQCAtNTAsNyArNTAs NyBAQCBwdWJsaWM6CiAKICAgICB2b2lkIHN0YXJ0KGNvbnN0IFN0cmluZyYgbWltZVR5cGUsIGNv bnN0IFdlYkNvcmU6OkludFJlY3QmKTsKICAgICB2b2lkIHVwZGF0ZVByb2dyZXNzKGZsb2F0KTsK LSAgICB2b2lkIGZpbmlzaChXZWJDb3JlOjpVUkwpOworICAgIHZvaWQgZmluaXNoKFdlYkNvcmU6 OlVSTCBkZXN0aW5hdGlvbkxvY2F0aW9uLCBjb25zdCBTdHJpbmcmIG9yaWdpbmFsRnJhZ21lbnRJ ZGVudGlmaWVyKTsKICAgICB2b2lkIGNhbmNlbCgpOwogICAgIHZvaWQgZmFpbChjb25zdCBXZWJD b3JlOjpSZXNvdXJjZUVycm9yJik7CiAK