Bug 90870 - REGRESSION(r109480): Form state for iframe content is not restored
Summary: REGRESSION(r109480): Form state for iframe content is not restored
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: History (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P1 Normal
Assignee: Kent Tamura
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-10 04:32 PDT by Kent Tamura
Modified: 2012-08-27 21:19 PDT (History)
5 users (show)

See Also:


Attachments
Repro. Put this to LayoutTests/fast/loader/. Requires iframe/@srcdoc. (977 bytes, text/html)
2012-07-10 04:32 PDT, Kent Tamura
no flags Details
Patch (8.94 KB, patch)
2012-07-11 00:43 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Patch 2 (8.91 KB, patch)
2012-08-23 17:58 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Patch for landing (8.87 KB, patch)
2012-08-27 20:36 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 2012-07-10 04:32:32 PDT
Created attachment 151438 [details]
Repro.  Put this to LayoutTests/fast/loader/.  Requires iframe/@srcdoc.

Regression by 
    http://trac.webkit.org/changeset/109480/trunk
    https://bugs.webkit.org/show_bug.cgi?id=79206

In r109480, I forgot to take care of documents in <iframe>s in a document loaded by FrameLoader::loadItem().
Comment 1 Kent Tamura 2012-07-11 00:43:26 PDT
Created attachment 151624 [details]
Patch
Comment 2 Kent Tamura 2012-07-11 00:47:30 PDT
Comment on attachment 151624 [details]
Patch

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

> Source/WebCore/loader/HistoryController.cpp:166
> -    if (item->isCurrentDocument(document)) {
> +    if (item->isCurrentDocument(document) && document->attached()) {

I'm not confident of this change.

HistoryController::saveDocumentState() is called twice for a subframe document when the main frame is navigated to another URL.

1. FrameLoader::detachChildren() of the parent FrameLoader
   FrameLoader::detachFromParent()
   FrameLoader::closeURL()
   HistoryController::saveDocumentState()

2. HTMLFrameOwnerElement::disconnectContentFrame
   FrameLoader::frameDetached()
   FrameLoader::detachFromParent()
   FrameLoader::closeURL()
   HistoryController::saveDocumentState()

Is it an expected behavior?
Comment 3 Kent Tamura 2012-08-23 17:58:39 PDT
Created attachment 160301 [details]
Patch 2

just rebase
Comment 4 jochen 2012-08-27 04:05:42 PDT
Comment on attachment 160301 [details]
Patch 2

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

> Source/WebCore/ChangeLog:29
> +        Added. This function checks the current HisotryItem is associated

spelling Hisotry

> Source/WebCore/loader/HistoryController.cpp:182
> +static inline bool isAssociatedToRequestedHisotryItem(const HistoryItem* current, Frame* frame, const HistoryItem* requested)

spelling "Hisotry"
Comment 5 Kent Tamura 2012-08-27 20:36:31 PDT
Created attachment 160891 [details]
Patch for landing

hisotry
Comment 6 WebKit Review Bot 2012-08-27 21:19:50 PDT
Comment on attachment 160891 [details]
Patch for landing

Clearing flags on attachment: 160891

Committed r126839: <http://trac.webkit.org/changeset/126839>
Comment 7 WebKit Review Bot 2012-08-27 21:19:54 PDT
All reviewed patches have been landed.  Closing bug.