Bug 64895 - Wrong URL loaded into child frame after back/forward navigation
Summary: Wrong URL loaded into child frame after back/forward navigation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nate Chapin
URL:
Keywords:
: 64402 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-07-20 14:11 PDT by Nate Chapin
Modified: 2011-08-17 13:18 PDT (History)
4 users (show)

See Also:


Attachments
patch (5.42 KB, patch)
2011-07-20 14:25 PDT, Nate Chapin
no flags Details | Formatted Diff | Diff
Patch for landing (5.56 KB, patch)
2011-07-22 11:14 PDT, Nate Chapin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nate Chapin 2011-07-20 14:11:11 PDT
Original report at http://code.google.com/p/chromium/issues/detail?id=59576.

Repro steps:
1. Display a web page
2. In the web page, display an iframe or object containing html
3. Navigate back & forward
4. In the same web page, display another iframe containing a different html page

The problem is in the logic in FrameLoader::loadURLIntoChildFrame to load a child frame from a HistoryItem instead of from the url.  I think we only want to load from a HistoryItem before the load event.

Note that this problem is prevented by the page cache because the old contents of the child frame are immediately present and the display of a  new frame will typically add a new Frame rather than overwrite.
Comment 1 Nate Chapin 2011-07-20 14:25:11 PDT
Created attachment 101509 [details]
patch
Comment 2 Mihai Parparita 2011-07-20 18:25:57 PDT
Comment on attachment 101509 [details]
patch

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

Is the current behavior (that the URL is restored for iframes for back-forward navigations) tested anywhere?

> LayoutTests/fast/loader/child-frame-add-after-back-forward.html:9
> +    layoutTestController.overridePreference('WebKitUsesPageCachePreferenceKey', 0);

You may want to add an unload handler too, so that the page cache is disabled when running the test in Safari too.
Comment 3 Nate Chapin 2011-07-21 15:13:05 PDT
(In reply to comment #2)
> (From update of attachment 101509 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=101509&action=review
> 
> Is the current behavior (that the URL is restored for iframes for back-forward navigations) tested anywhere?

Yeah, if I comment out the if() statement and everything inside, 2 tests each in fast/history and http/tests/navigation fail.

> 
> > LayoutTests/fast/loader/child-frame-add-after-back-forward.html:9
> > +    layoutTestController.overridePreference('WebKitUsesPageCachePreferenceKey', 0);
> 
> You may want to add an unload handler too, so that the page cache is disabled when running the test in Safari too.

Good point.
Comment 4 Nate Chapin 2011-07-22 11:14:03 PDT
Created attachment 101743 [details]
Patch for landing
Comment 5 WebKit Review Bot 2011-07-22 12:02:11 PDT
Comment on attachment 101743 [details]
Patch for landing

Clearing flags on attachment: 101743

Committed r91583: <http://trac.webkit.org/changeset/91583>
Comment 6 WebKit Review Bot 2011-07-22 12:02:15 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Alexey Proskuryakov 2011-08-17 13:18:15 PDT
*** Bug 64402 has been marked as a duplicate of this bug. ***