Distinct redirects from the same link do not create distinct history items

• Component: History
• Status: NEW ---

[Cameron Zwarich (cpst)]

If you click "Random Comic" on http://catandgirl.com/ and then click it again, it does not create a new history item. Thus, going back goes back the front page, no matter how many random comics were viewed. I suspect this is because the random comic link is the same on each page, but redirects the user to a different page each time.

[Cameron Zwarich (cpst)]

The problem is that shouldTreatURLAsSameAsCurrent() is saying that the "random comic" link is the same as the current link. The check in shouldTreatURLAsSameAsCurrent() is

    return url == m_currentHistoryItem->url() || url == m_currentHistoryItem->originalURL();

If I remove the second part of this test, then the bug is fixed, with two caveats:

1) There is a back/forward history item, but no item in the history item.

2) If I click the link while it is redirecting, it seems I can make URLs appear in the history of the form "http://cantandgirl.com./..." with a period after ".com". The URL works, but I am pretty sure it isn't the URL being given by the server.

The check has been there at least since revision 2963:

http://trac.webkit.org/changeset/2963

If no one responds with a reason why this change is obviously a bad idea, I will just submit a patch with test cases for review.

[Cameron Zwarich (cpst)]

Created attachment 21615 [details]
Proposed patch

Here it is, with a test case. The problem with the History menu isn't fixed, but I can work on that next if this change is acceptable.

[Cameron Zwarich (cpst)]

*** Bug 7814 has been marked as a duplicate of this bug. ***

[Brady Eidson]

Comment on attachment 21615 [details]
Proposed patch

So, I *think* this might break history with certain types of redirect chains.

Imagine visiting URL A, which redirects to URL B, which redirects back to A.  With this patch, I think the single navigation would result in 2 history items, which isn't correct.

I discussed this with Cameron on irc, and I he's trying to write a test to replicate my theoretical scenario

[Cameron Zwarich (cpst)]

Created attachment 21654 [details]
Test case

Here is a test case for the existing behaviour. It passes with my patch applied. I put it up for review so I can land the test before working on this bug some more.

I am only dumping the back/forward list; is there any way to also dump global history? Since that is one of the issues with this bug, I would like to be able to test it.

Interestingly enough, this test case seems to expose a bug in WebKit. If I add a layoutTestController.queueBackNavigation(1) to the final page, it performs no navigation. Should I file this bug separately?

[Darin Adler]

Comment on attachment 21654 [details]
Test case

r=me

[Cameron Zwarich (cpst)]

Landed in r34525.

[Cameron Zwarich (cpst)]

Comment on attachment 21654 [details]
Test case

Clearing review.

[Graham Perrin]

Alongside this bug 19422 I am reading 
https://bugs.webkit.org/show_bug.cgi?id=18701

> Clicking on redirect link stores wrong entry in history

[Alexey Proskuryakov]

This is still an issue in Safari 5. Brady, do you still think that the patch is incorrect, given the test case?