RESOLVED FIXED 191141
Forward original fragment identifier into System Preview 
https://bugs.webkit.org/show_bug.cgi?id=191141
Summary Forward original fragment identifier into System Preview
Dean Jackson
Reported 2018-10-31 17:25:51 PDT
Forward original fragment identifier into System Preview
Attachments
Patch (4.64 KB, patch)
2018-10-31 17:32 PDT, Dean Jackson 		wenson_hsieh: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2018-10-31 17:29:41 PDT
<rdar://problem/45717542>
Dean Jackson
Comment 2 2018-10-31 17:32:25 PDT
Created attachment 353561 [details] Patch
Wenson Hsieh
Comment 3 2018-10-31 17:45:23 PDT
Comment on attachment 353561 [details] 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?
Dean Jackson
Comment 4 2018-10-31 17:47:13 PDT
Comment on attachment 353561 [details] 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).
Dean Jackson
Comment 5 2018-10-31 17:49:32 PDT
Comment on attachment 353561 [details] 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.
Dean Jackson
Comment 6 2018-10-31 18:03:41 PDT
Committed r237664: <https://trac.webkit.org/changeset/237664>
Note You need to log in before you can comment on or make changes to this bug.