RIM PR: 160585 way to reproduce: try to load an digest http authenticated page which has a subresource, e.g. an img tag pointing to a png file which needs the same authentication. the auth challenge dialog would pop up twice, even you've entered the right credentials for the main resource. root cause: we load store the credentials in handleNotifyDone, after finish loading the whole main resource, which is too late. We should do it as soon as we receives the headers which indicate that we've passed the authentication. Then we can find the credential information from CredentialStorage when loading subresource.
Created attachment 144357 [details] Patch
LGTM
Comment on attachment 144357 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144357&action=review Looks good, but can maybe cleaned up some more. > Source/WebCore/ChangeLog:14 > + No new tests because of no behavior changed. No new tests since there is no change in behavior. > Source/WebCore/platform/network/blackberry/NetworkJob.cpp:214 > + // the saved credentials. The comment is quite obvious, and the code below it is readable, maybe not needed. > Source/WebCore/platform/network/blackberry/NetworkJob.cpp:218 > + purgeCredentials(); Is it useful to log the fact that there was an error? How about dealing with isError case first, you do not need the ! operation in that case.
Created attachment 144438 [details] Patch
Created attachment 144443 [details] Patch
Created attachment 144446 [details] Patch
(In reply to comment #6) > Created an attachment (id=144446) [details] > Patch after talk with Joe, we agreed to keep the exact same code as before, just move them to notifyStatusReceived. We need to think more before being sure we are right about that, such as 500 or 404 or something else, and we need have more tests. For now the way won't be perfect but at least we won't make things worse.
Hi Jonathan, (In reply to comment #7) > (In reply to comment #6) > > Created an attachment (id=144446) [details] [details] > > Patch > > after talk with Joe, we agreed to keep the exact same code as before, just move them to notifyStatusReceived. We need to think more before being sure we are right about that, such as 500 or 404 or something else, and we need have more tests. For now the way won't be perfect but at least we won't make things worse. So I made my comment because if the isError is true but isUnauthorized is false, we would not report/log this error, isn't that losing information? Hence my log suggestion, why not always log if there is an error? Cheers, Rob.
(In reply to comment #8) > So I made my comment because if the isError is true but isUnauthorized is false, we would not report/log this error, isn't that losing information? Hence my log suggestion, why not always log if there is an error? Of coures we report that error. If isError is true but isUnauthorized is false, it just falls through to existing error handling and doesn't touch the credentials at all.
Comment on attachment 144446 [details] Patch Looks good.
Comment on attachment 144446 [details] Patch Clearing flags on attachment: 144446 Committed r118799: <http://trac.webkit.org/changeset/118799>
All reviewed patches have been landed. Closing bug.