Bug 64895

Summary: Wrong URL loaded into child frame after back/forward navigation
Product: WebKit Reporter: Nate Chapin <japhet>
Component: WebCore Misc.Assignee: Nate Chapin <japhet>
Status: RESOLVED FIXED    
Severity: Normal CC: beidson, mihaip, webkit.review.bot, xt6.thomas.genin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
none
Patch for landing none

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. ***