Bug 95488 - [BlackBerry] when one of multiple tabs uses authentication, user can get the auth dialog while the other tab has focus.
Summary: [BlackBerry] when one of multiple tabs uses authentication, user can get the ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit BlackBerry (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Other
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-30 12:47 PDT by Lyon Chen
Modified: 2012-09-07 10:12 PDT (History)
8 users (show)

See Also:


Attachments
First patch for 95488 (18.71 KB, patch)
2012-09-06 19:26 PDT, Lyon Chen
yong.li.webkit: review+
Details | Formatted Diff | Diff
Patch updated based on Yong's comments. (19.06 KB, patch)
2012-09-07 08:14 PDT, Lyon Chen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Lyon Chen 2012-08-30 12:47:26 PDT
Observed Result:
Constantly being prompted for username/password when one of multiple tabs uses
NTLM style authentication

Expected Result:
Should not be prompted for username/password more than once when using multiple
tabs

Steps To Reproduce:
open a tab and browse to a NTLM authenticated site
http://lifeatrim.rim.net/

enter the rim account name and password to login to the site
open a new tab and browse to another site such as www.rim.com
Comment 1 Lyon Chen 2012-08-30 12:48:00 PDT
Add [BlackBerry] to title.
Comment 2 Lyon Chen 2012-08-30 12:53:47 PDT
Add Yong, Joe, and Rob.
Comment 3 Lyon Chen 2012-09-06 19:26:14 PDT
Created attachment 162653 [details]
First patch for 95488
Comment 4 Yong Li 2012-09-07 07:38:30 PDT
Comment on attachment 162653 [details]
First patch for 95488

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

> Source/WebCore/ChangeLog:8
> +
> +        Add AuthenticationChallengeClient interface to support asynchronous

Better put the RIM PR number here, so we will be able to link the commit to the PR

> Source/WebCore/ChangeLog:16
> +        Note: this patch itself will not fix this bug, but it prepares the base
> +        for the next patch by making the authentication asynchronous to NetworkJob
> +        and MediaPlayerPrivateBlackBerry.

why not put them together? That will make it easier for reviewer to understand how it will work

> Source/WebCore/ChangeLog:18
> +        No new tests as there is no change of behavior.

Doing it in async mode is a big behavior change, with your next patch :) We don't have to upstream a layout test for our platform-specific test though.

> Source/WebCore/platform/network/blackberry/NetworkJob.cpp:832
> +    if (result != AuthenticationChallengeSuccess
> +            || protectionSpace.host().isEmpty()
> +            || !url.isValid()) {

style: using one line is good enough?
Comment 5 Yong Li 2012-09-07 07:40:57 PDT
Comment on attachment 162653 [details]
First patch for 95488

please put "internally reviewed by Joe Mason" in the change log. otherwise r+
Comment 6 Lyon Chen 2012-09-07 08:14:57 PDT
Created attachment 162777 [details]
Patch updated based on Yong's comments.

Yong: thanks, this is the updated patch, please check whether it is OK or need further improvement.
Comment 7 WebKit Review Bot 2012-09-07 10:12:55 PDT
Comment on attachment 162777 [details]
Patch updated based on Yong's comments.

Clearing flags on attachment: 162777

Committed r127884: <http://trac.webkit.org/changeset/127884>
Comment 8 WebKit Review Bot 2012-09-07 10:12:58 PDT
All reviewed patches have been landed.  Closing bug.