if wrong credential information is provided by user for the first time, some site would return 401 again to challenge user to re-enter the credential, until user correctly login or press cancel. According to our implementation for now, NetworkJob would not start a new request when the m_currentWebChallenge in its related resourceHandle is not null. It won't send out a new request when user leaves both username and password empty then press enter.
Created attachment 133403 [details] Patch
Comment on attachment 133403 [details] Patch Looks good.
Comment on attachment 133403 [details] Patch Clearing flags on attachment: 133403 Committed r111810: <http://trac.webkit.org/changeset/111810>
All reviewed patches have been landed. Closing bug.
Reopening to attach new patch.
Created attachment 133796 [details] Patch
hi Rob, do you have any comment on this? :)
Comment on attachment 133796 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133796&action=review > Source/WebCore/ChangeLog:8 > + Fixed a regression introduced by last patch to fix this PR (git SHA: d9a7ccc1a98e), I dont think this is useful for webkit community, to mention internal SHAs. I propose to do what chromium does, refer to our internal bug (like PR 10777), and in the internal bug you can refer to SHAs. However if you want to refer to a previous upstream webkit changeset you did (like r110000), that would be fine.
Created attachment 134575 [details] Patch
Comment on attachment 134575 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=134575&action=review Still some issues :) > Source/WebCore/ChangeLog:5 > + RIM PR: 145660 It is better to put this below the Reviewed By line. In fact cq+ might not work if it is strict about the format. > Source/WebCore/ChangeLog:9 > + Fixed a regression introduced by last patch to fix this PR (r111810), we should Better rephrase: Fixed a regression introduced by r111810, we should > Source/WebCore/ChangeLog:13 > + No new tests. Why no new tests? > Source/WebKit/blackberry/ChangeLog:9 > + Fixed a regression introduced by last patch to fix this PR (r111810), which Same as above.
Created attachment 134955 [details] Patch
Comment on attachment 134955 [details] Patch Looks good.
Comment on attachment 134955 [details] Patch Clearing flags on attachment: 134955 Committed r112794: <http://trac.webkit.org/changeset/112794>
Re-opening: these changes broke digest auth. Going to revert them until we can figure out the problem.
Created attachment 138464 [details] revert patch
Comment on attachment 138464 [details] revert patch Why remove the tests?
(In reply to comment #17) > (From update of attachment 138464 [details]) > Why remove the tests? I just reverted the whole thing. This is "git revert" on all three patches, squashed.
Created attachment 138552 [details] revert all but tests Here's the same patch without the tests removed. r+ whichever you prefer and r- the other one.
Created attachment 138570 [details] revert all but the tests Updated patch that reverts all but the tests.
Comment on attachment 138570 [details] revert all but the tests Clearing flags on attachment: 138570 Committed r115104: <http://trac.webkit.org/changeset/115104>
No, those were reverts, so this is still open.
Created attachment 144364 [details] Patch
Comment on attachment 144364 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144364&action=review Looks good but please fix my issues before landing. > Source/WebCore/ChangeLog:9 > + issue for BlackBerry porting get identified and fixed. It is better to keep the original ChangeLog messages. Or keep them and append these two lines too. > Tools/DumpRenderTree/blackberry/DumpRenderTree.cpp:130 > + return "<unknown>"; This needs to be implemented? If so add a TODO. > Tools/DumpRenderTree/blackberry/DumpRenderTree.cpp:877 > + return true; Is this tested? Do you think we will need to change some test results?
Comment on attachment 144364 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144364&action=review >> Tools/DumpRenderTree/blackberry/DumpRenderTree.cpp:877 >> + return true; > > Is this tested? Do you think we will need to change some test results? yes it's been tested with http/tests/loading/basic-auth-resend-wrong-credentials.html and http/tests/loading/basic-credentials-sent-automatically.html I don't think we need to change test results because that the purpose of reverting this patch is for the infinite loop challenge when loading subresources, it has nothing to do with the DRT implementation. And the tests work fine for now.
Created attachment 144426 [details] Patch
Comment on attachment 144426 [details] Patch Clearing flags on attachment: 144426 Committed r118719: <http://trac.webkit.org/changeset/118719>