Summary: | Windows client needs updating when live iframe element is moved between pages | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jenn Braithwaite <jennb> | ||||||||
Component: | Frames | Assignee: | Jenn Braithwaite <jennb> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | aroben, commit-queue, dimich | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | Windows 7 | ||||||||||
Attachments: |
|
Description
Jenn Braithwaite
2010-09-30 10:02:48 PDT
(In reply to comment #1) > I believe bug 44713 is the Mac equivalent. 44713 has a slightly different issue. Mac has the correct WebView for the frame after reparenting, but the resource is only known to the WebView of the former page. Knowledge of the frame's resources need to be transferred over from the old WebView to the frame's current WebView to fix 44713. After this bug is fixed for Windows, we also have to fix 44713 for Windows (and other platforms). Created attachment 71226 [details]
patch
Comment on attachment 71226 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=71226&action=review > WebKit/win/WebCoreSupport/WebFrameLoaderClient.cpp:742 > void WebFrameLoaderClient::didTransferChildFrameToNewDocument(Page*) > { > + WebView* webView = m_webFrame->webView(); > + Frame* coreFrame = core(m_webFrame); > + Frame* coreParentFrame = coreFrame->tree()->parent(); > + WebView* parentWebView = kit(coreParentFrame)->webView(); > + > + if (webView != parentWebView) > + m_webFrame->setWebView(parentWebView); > + > + ASSERT(m_webFrame->webView()->page() == coreFrame->page()); > } Does the passed-in Page correspond to the new document? If so you can just do: WebView* newWebView = core(page); (The name of this function is confusing; it's talking about moving a frame between pages, but says "document" instead of "page"!) (In reply to comment #5) > (From update of attachment 71226 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=71226&action=review > > > WebKit/win/WebCoreSupport/WebFrameLoaderClient.cpp:742 > > void WebFrameLoaderClient::didTransferChildFrameToNewDocument(Page*) > > { > > + WebView* webView = m_webFrame->webView(); > > + Frame* coreFrame = core(m_webFrame); > > + Frame* coreParentFrame = coreFrame->tree()->parent(); > > + WebView* parentWebView = kit(coreParentFrame)->webView(); > > + > > + if (webView != parentWebView) > > + m_webFrame->setWebView(parentWebView); > > + > > + ASSERT(m_webFrame->webView()->page() == coreFrame->page()); > > } > > Does the passed-in Page correspond to the new document? If so you can just do: > > WebView* newWebView = core(page); > > (The name of this function is confusing; it's talking about moving a frame between pages, but says "document" instead of "page"!) The passed in page is the old page. I will be removing that argument after bug 44713 is fixed as it turns out that argument is not needed for the fix. Comment on attachment 71226 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=71226&action=review >>> WebKit/win/WebCoreSupport/WebFrameLoaderClient.cpp:742 >>> } >> >> Does the passed-in Page correspond to the new document? If so you can just do: >> >> WebView* newWebView = core(page); >> >> (The name of this function is confusing; it's talking about moving a frame between pages, but says "document" instead of "page"!) > > The passed in page is the old page. I will be removing that argument after bug 44713 is fixed as it turns out that argument is not needed for the fix. This code will crash if coreFrame doesn't have a parent frame, or that parent frame doesn't have a WebFrame associated with it. You should either add some assertions or some null-checks. Other than that this looks fine. Created attachment 71605 [details]
updated patch
Revised to update the webview without referring to the parent frame as it's possible to find the webview directly from Page. Same result. Simpler code.
Comment on attachment 71605 [details] updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=71605&action=review > WebKit/win/WebCoreSupport/WebFrameLoaderClient.cpp:737 > void WebFrameLoaderClient::didTransferChildFrameToNewDocument(Page*) > { > + Frame* coreFrame = core(m_webFrame); > + if (m_webFrame->webView()->page() != coreFrame->page()) > + m_webFrame->setWebView(kit(coreFrame->page())); > } > This definitely looks better. It's possible for a Frame not to have a Page, and for a WebFrame not to have a WebView, though. Could we run into that situation here? Created attachment 71613 [details]
updated patch again
Revised code to work even if Frame has no Page - kit(Page) handles this case - and if WebFrame has no WebView.
Comment on attachment 71613 [details] updated patch again Clearing flags on attachment: 71613 Committed r70376: <http://trac.webkit.org/changeset/70376> All reviewed patches have been landed. Closing bug. |