[Webkit-unassigned] [Bug 137302] location.href is not updated on a redirect with a custom protocol

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=137302

--- Comment #14 from Benjamin Poulain <benjamin at webkit.org> ---
Comment on attachment 240670
  --> https://bugs.webkit.org/attachment.cgi?id=240670
Patch

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

> > Weird number of spaces at line-start
> What does it mean?
> I did Xcode style indentation there that you've asked there.

Don't worry, the style-checker does not know about Objective-C indentation. A code style "error" is a hint for reviewers.

> Source/WebCore/ChangeLog:13
> +        No new tests, just an ASSERT removal.

(In reply to comment #10)
> > You can only use this excuse for tests when you believe there is no observable difference after your change.
> > In this case, we would crash in Debug due to the assertion so it is observable and should have a test.
> 
> I couldn't find a place in code where I could put a simple unit test for
> CredentialStorage::findDefaultProtectionSpaceForURL, so I've just changed
> the log message.

Honestly I don't understand why you remove the assertion if you cannot find a case that hits it...

> Source/WebKit2/ChangeLog:10
> +        This was happening, because WKCustomProtocolLoader didn't forward

Extra coma "happening, because" -> happening because

> Source/WebKit2/UIProcess/Network/CustomProtocols/mac/CustomProtocolManagerProxyMac.mm:113
> +    if (redirectResponse == nil)

WebKit style is:
    if (!redirectResponse)

> Source/WebKit2/UIProcess/Network/CustomProtocols/mac/CustomProtocolManagerProxyMac.mm:121
> +    // Abandon the current connection, a new protocol connection for "request" will be initiated later.
> +    _customProtocolManagerProxy->stopLoading(_customProtocolID);

It is not clear to me why this is correct. NSURLConnection never calls connectionDidFinishLoading: for redirection? This interface should behave the same.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20141030/75fc4028/attachment-0002.html>