Bug 63777 - location.replace with a hash change does not update the history entry
Summary: location.replace with a hash change does not update the history entry
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: History (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Mihai Parparita
URL:
Keywords:
: 57979 59195 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-06-30 18:20 PDT by Mihai Parparita
Modified: 2011-07-01 15:03 PDT (History)
6 users (show)

See Also:


Attachments
Patch (6.56 KB, patch)
2011-06-30 18:21 PDT, Mihai Parparita
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-01 (1.31 MB, application/zip)
2011-06-30 21:40 PDT, WebKit Review Bot
no flags Details
Patch (6.60 KB, patch)
2011-06-30 22:18 PDT, Mihai Parparita
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mihai Parparita 2011-06-30 18:20:00 PDT
location.replace with a hash change does not update the history entry
Comment 1 Mihai Parparita 2011-06-30 18:21:31 PDT
Created attachment 99411 [details]
Patch
Comment 2 Mihai Parparita 2011-06-30 18:23:40 PDT
The test case passes in Firefox, I will verify the behavior in IE shortly.

Fixes http://crbug.com/86000 and http://crbug.com/78485.
Comment 3 Mihai Parparita 2011-06-30 18:25:10 PDT
*** Bug 57979 has been marked as a duplicate of this bug. ***
Comment 4 Mihai Parparita 2011-06-30 18:27:50 PDT
*** Bug 59195 has been marked as a duplicate of this bug. ***
Comment 5 Mihai Parparita 2011-06-30 18:35:45 PDT
IE behaves the same as Firefox.
Comment 6 WebKit Review Bot 2011-06-30 21:40:20 PDT
Comment on attachment 99411 [details]
Patch

Attachment 99411 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8959920

New failing tests:
fast/loader/crash-replacing-location-before-load.html
Comment 7 WebKit Review Bot 2011-06-30 21:40:25 PDT
Created attachment 99424 [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 8 Mihai Parparita 2011-06-30 22:18:03 PDT
Created attachment 99426 [details]
Patch
Comment 9 Darin Fisher (:fishd, Google) 2011-06-30 22:53:27 PDT
Comment on attachment 99426 [details]
Patch

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

R=me

> Source/WebCore/loader/HistoryController.cpp:514
> +    if (m_currentItem) {

I notice that recursiveUpdateForSameDocumentNavigation() can assign m_currentItem if
there is a non-null m_provisionalItem.  I just want to make sure that it makes sense
to work with m_currentItem after calling recursiveUpdateForSameDocumentNavigation()
versus before.
Comment 10 Mihai Parparita 2011-07-01 14:18:55 PDT
(In reply to comment #9)
> I notice that recursiveUpdateForSameDocumentNavigation() can assign m_currentItem if
> there is a non-null m_provisionalItem.  I just want to make sure that it makes sense
> to work with m_currentItem after calling recursiveUpdateForSameDocumentNavigation()
> versus before.

I think so, it seems reasonable to want the the current history item to reflect the current URL, instead of having those changes be clobbered when a provisional load is committed.
Comment 11 WebKit Review Bot 2011-07-01 15:02:57 PDT
Comment on attachment 99426 [details]
Patch

Clearing flags on attachment: 99426

Committed r90281: <http://trac.webkit.org/changeset/90281>
Comment 12 WebKit Review Bot 2011-07-01 15:03:04 PDT
All reviewed patches have been landed.  Closing bug.