[Webkit-unassigned] [Bug 99352] [GTK] [WebKit2] Add an 'authenticate' signal to WebKitWebView

[bugzilla-daemon at webkit.org]

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





--- Comment #52 from Brian Holt <brian.holt at samsung.com>  2013-08-07 06:24:18 PST ---
(From update of attachment 207922)
View in context: https://bugs.webkit.org/attachment.cgi?id=207922&action=review

>> Source/WebKit2/UIProcess/API/gtk/WebKitAuthenticationRequest.cpp:51
>> + * WebKitAuthenticationRequest object.
> 
> I think this should be in the signal documentation.

Since all this information is already in the signal documentation I'll just remove this paragraph.

>> Source/WebKit2/UIProcess/API/gtk/WebKitAuthenticationRequest.cpp:202
>> +    return request->priv->realm.data();
> 
> The realm doesn't change, so I think it makes sense to do this:
> 
> if (request->priv->realm.isNull())
>     request->priv->realm = request->priv->authenticationChallenge->protectionSpace()->realm().utf8();
> 
> This will prevent an application that caches the realm from crashing if something else calls the method again.

I didn't realise calling it again would crash, so thanks for spotting that.

>> Source/WebKit2/UIProcess/API/gtk/WebKitAuthenticationRequest.cpp:254
>> +}
> 
> We can't really expose this properly. The reason is that the authentication request we get from soup only exposes whether or not the challenge is a retry. This will always be one or zero, regardless of how many times it has failed. So I think you should either remove this method or turn it into webkit_authentication_request_is_retry.
> 
> See AuthenticationChallenge::AuthenticationChallenge in AuthenticationChallengeSoup.cpp.

I realised that when I was writing the unit test and its been on my mind. EFL exposes a is_retry function using soup underneath as well.

>> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebView.cpp:920
>> +    g_assert(!webkit_web_view_get_title(test->m_webView));
> 
> What exactly happens in this case? How does the load finish? load failed is emitted with cancel error? I that's is the case we should test that, connect to load-failed and check the given error is cancel. This behaviour should probably be documented in the webkit_authentication_request_cancel API doc too.

Will do, thats a really good point.

>> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebView.cpp:1405
>> +    AuthenticationTest::add("WebKitWebView", "authentication", testWebViewAuthenticationRequest);
> 
> Any reason you added this at the beginning? It doesn't really matter, but we usually add new tests at the end

I thought it was meant to be alphabetical...

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.