Summary: | PCM: Allow measurement of links in nested, cross-site iframes | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Pranay Prabhat <pranay.prabhat> | ||||||||
Component: | WebKit Misc. | Assignee: | John Wilander <wilander> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | achristensen, cdumez, changseok, esprehn+autocc, ews-watchlist, gyuyoung.kim, webkit-bug-importer, wilander | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | Safari 14 | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Pranay Prabhat
2021-08-17 14:43:04 PDT
Created attachment 440110 [details]
Patch
Comment on attachment 440110 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=440110&action=review > Source/WebCore/html/HTMLAnchorElement.cpp:446 > + if (frame->isMainFrame()) > + mainDocumentRegistrableDomain = RegistrableDomain { document().url() }; > + else if (auto mainDocument = frame->mainFrame().document()) > + mainDocumentRegistrableDomain = RegistrableDomain { mainDocument->url() }; I think you can simplify this because Frame::mainFrame() returns itself if it's the main frame. (In reply to Alex Christensen from comment #3) > Comment on attachment 440110 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=440110&action=review > > > Source/WebCore/html/HTMLAnchorElement.cpp:446 > > + if (frame->isMainFrame()) > > + mainDocumentRegistrableDomain = RegistrableDomain { document().url() }; > > + else if (auto mainDocument = frame->mainFrame().document()) > > + mainDocumentRegistrableDomain = RegistrableDomain { mainDocument->url() }; > > I think you can simplify this because Frame::mainFrame() returns itself if > it's the main frame. Thanks! Will fix before landing. Comment on attachment 440110 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=440110&action=review >>> Source/WebCore/html/HTMLAnchorElement.cpp:446 >>> + mainDocumentRegistrableDomain = RegistrableDomain { mainDocument->url() }; >> >> I think you can simplify this because Frame::mainFrame() returns itself if it's the main frame. > > Thanks! Will fix before landing. You don't even need frames, document().topDocument().url() (In reply to Chris Dumez from comment #5) > Comment on attachment 440110 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=440110&action=review > > >>> Source/WebCore/html/HTMLAnchorElement.cpp:446 > >>> + mainDocumentRegistrableDomain = RegistrableDomain { mainDocument->url() }; > >> > >> I think you can simplify this because Frame::mainFrame() returns itself if it's the main frame. > > > > Thanks! Will fix before landing. > > You don't even need frames, document().topDocument().url() I looked at the implementation of topDocument() and this part has me worried about potential misuse vectors: if (backForwardCacheState() == NotInBackForwardCache && !m_renderTreeBeingDestroyed) { if (!m_frame) return const_cast<Document&>(*this); // This should always be non-null. Document* mainFrameDocument = m_frame->mainFrame().document(); return mainFrameDocument ? *mainFrameDocument : const_cast<Document&>(*this); } I'll go with if (auto mainDocument = frame->mainFrame().document()). Created attachment 440122 [details]
Patch
Created attachment 440215 [details]
Patch
Fixed test for iOS. Committed r283593 (242549@main): <https://commits.webkit.org/242549@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 440215 [details]. |