[webkit-reviews] review granted: [Bug 34382] When a live iframe element is moved between pages, it still depends on the old page. : [Attachment 48547] Same patch, with style fix.

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has granted Dmitry Titov
<dimich at chromium.org>'s request for review:
Bug 34382: When a live iframe element is moved between pages, it still depends
on the old page.
https://bugs.webkit.org/show_bug.cgi?id=34382

Attachment 48547: Same patch, with style fix.
https://bugs.webkit.org/attachment.cgi?id=48547&action=review

------- Additional Comments from David Levin <levin at chromium.org>

1. I would remove all notImplemented(); I believe these imply that there is a
missing implementation, but that is not the case here. The ports with empty
methods simply don't need an implementation as far as we know.
2. I'm concerned about the change in WebKit/qt/Api/qwebframe.h. Does this mean
that the public api in qt is changing? It looks like Kenneth Rohde Christiansen
< kenneth at webkit.org> has been looking at these changes. Perhaps you could get
him to look at this part of your patch.


r=me after kenneth gives an ok (and it would be nice to address the other
things mentioned).


> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> +2010-02-10  Dmitry Titov  <dimich at chromium.org>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   When a live iframe element is moved between pages, it still depends
on the old page.
> +	   https://bugs.webkit.org/show_bug.cgi?id=34382
> +
> +	   Test: fast/frames/iframe-reparenting-new-page.html
> +
> +	   * html/HTMLFrameElementBase.cpp:
> +	   (WebCore::HTMLFrameElementBase::setName):
> +	   Move the code setting the frame name into a separate function.
> +
> +	   (WebCore::HTMLFrameElementBase::setNameAndOpenURL):
> +	   (WebCore::HTMLFrameElementBase::updateOnReparenting):
> +	   Called on the frame that was just re-parented and inserted into
another Document.
> +	   Simply invoke Frame::updateOnreparenting(newParentFrame);

updateOnReparenting
	^ capital R



In WebKit/chromium/ChangeLog
> +	    if Frame moves between Pages the client of corresponding WebFrame
> +	    should be replaced as well.
add a "," after Pages.