Bug 191141 – Forward original fragment identifier into System Preview

• Component: New Bugs
• Status: RESOLVED FIXED

[Dean Jackson]

Forward original fragment identifier into System Preview

[Radar WebKit Bug Importer]

<rdar://problem/45717542>

[Dean Jackson]

Created attachment 353561 [details]
Patch

[Wenson Hsieh]

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 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 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]

Committed r237664: <https://trac.webkit.org/changeset/237664>