Closed
Bug 1037687
Opened 10 years ago
Closed 9 years ago
Investigate if iframe/frame/object in shadow dom hosted by in-tree element should be loaded
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla39
Tracking | Status | |
---|---|---|
firefox39 | --- | fixed |
People
(Reporter: smaug, Assigned: smaug)
References
Details
Attachments
(2 files)
16.03 KB,
patch
|
wchen
:
review+
|
Details | Diff | Splinter Review |
18.85 KB,
patch
|
wchen
:
review+
|
Details | Diff | Splinter Review |
So far I haven't found any spec defining they should load, but that is probably a bug in HTML or DOM or Shadow DOM spec.
Comment 1•9 years ago
|
||
We'd like loading iframes if possible. How hard is this to implement?
Flags: needinfo?(bugs)
Assignee | ||
Comment 2•9 years ago
|
||
Making iframe to load should be close to trivial, but it isn't clear to me how session history should work. What kind of behavior would gaia prefer for session history? Should the pages inside shadow iframe participate session history?
Flags: needinfo?(bugs)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → bugs
Assignee | ||
Comment 3•9 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=f8bf71d85605
Attachment #8567157 -
Flags: review?(wchen)
Comment 4•9 years ago
|
||
Comment on attachment 8567157 [details] [diff] [review] v1 Review of attachment 8567157 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/nsFrameLoader.cpp @@ +1191,5 @@ > otherChildDocument->GetParentDocument(); > > // Make sure to swap docshells between the two frames. > + nsIDocument* ourDoc = ourContent->GetComposedDoc(); > + nsIDocument* otherDoc = otherContent->GetComposedDoc(); I think we also want to do replace GetCurrentDoc() with GetComposedDoc() in nsFrameLoader::SwapWithOtherRemoteLoader
Attachment #8567157 -
Flags: review?(wchen) → review+
Assignee | ||
Comment 5•9 years ago
|
||
ah yes. (I was thinking whether to change swap implementations at all.)
Assignee | ||
Comment 6•9 years ago
|
||
Ok, that particular case was easy, and defined in the spec. filed https://www.w3.org/Bugs/Public/show_bug.cgi?id=28086 for a related case which is not defined in the spec.
Attachment #8568260 -
Flags: review?(wchen)
Assignee | ||
Comment 7•9 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=ddf22e4011a3
Assignee | ||
Comment 8•9 years ago
|
||
Look for ShouldExposeChildWindow in the v2
Comment 9•9 years ago
|
||
Comment on attachment 8568260 [details] [diff] [review] v2 Review of attachment 8568260 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/WindowNamedPropertiesHandler.cpp @@ +21,5 @@ > { > + nsCOMPtr<nsPIDOMWindow> piWin = do_QueryInterface(aChild); > + NS_ENSURE_TRUE(piWin, false); > + Element* e = piWin->GetFrameElementInternal(); > + if (e && e->IsInShadowTree()) { If we want to match the spec language here (which uses "in a document") [1], we should use !e->IsInUncomposedDoc(). Although if you do decide to change it, it should also include a comment mentioning that the check is to ensure shadow DOM content is not exposed. The shadow DOM spec also mentions named properties but it doesn't actually say anything useful: "Window object named properties [HTML] must access the nodes in the document tree." (doesn't say that it should not access nodes in a shadow tree) [1] https://html.spec.whatwg.org/multipage/browsers.html#child-browsing-context
Attachment #8568260 -
Flags: review?(wchen) → review+
Assignee | ||
Comment 10•9 years ago
|
||
(In reply to William Chen [:wchen] from comment #9) > ::: dom/base/WindowNamedPropertiesHandler.cpp > @@ +21,5 @@ > > { > > + nsCOMPtr<nsPIDOMWindow> piWin = do_QueryInterface(aChild); > > + NS_ENSURE_TRUE(piWin, false); > > + Element* e = piWin->GetFrameElementInternal(); > > + if (e && e->IsInShadowTree()) { > > If we want to match the spec language here (which uses "in a document") [1], > we should use !e->IsInUncomposedDoc(). Although if you do decide to change > it, it should also include a comment mentioning that the check is to ensure > shadow DOM content is not exposed. Well, you don't get here if you're dealing with nodes not in composed doc. So in practice IsInShadowTree and !IsInUncomposedDoc() should have the same effect.
Assignee | ||
Comment 11•9 years ago
|
||
not sure which one is more understandable.
Assignee | ||
Comment 12•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/53365d1c2b7f
https://hg.mozilla.org/mozilla-central/rev/53365d1c2b7f
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Comment 14•9 years ago
|
||
Wow thanks guys, you're machine-like :)
You need to log in
before you can comment on or make changes to this bug.
Description
•