Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shadow DOM and the Fullscreen API #180

Closed
hayatoito opened this issue Jul 6, 2015 · 20 comments
Closed

Shadow DOM and the Fullscreen API #180

hayatoito opened this issue Jul 6, 2015 · 20 comments

Comments

@hayatoito
Copy link
Contributor

Migrated from: https://www.w3.org/Bugs/Public/show_bug.cgi?id=27379

See also https://www.w3.org/Bugs/Public/show_bug.cgi?id=26934#c2

@andyearnshaw
Copy link

What are the questions that need to be asked here? Chrome already seems to support setting a shadowy element to fullscreen as long as there's no fullscreen element already.

Here's a use case I have, maybe it will help with figuring this out. I have a shadow root on a {{}} element which could contain a HTML5 video or a YouTube embed (iframe), and a set of custom controls. Both the controls and the video are wrapped in another element. If a user clicks the "go fullscreen" button on those controls, I want the wrapping element to go full screen. Setting the wrapping element to full screen seems to make more sense than the custom element because external styles won't apply to the wrapping element inside the shadow tree, so we can (more or less) guarantee that it will look right.

document.fullscreenElement should probably reference the host, similar to how an iframe would be returned if an element in its document is set to fullscreen. I guess the problem there is, how do you tell which element in the shadow tree is set to fullscreen if there might be more than one? matches(':fullscreen'), maybe?

@annevk annevk changed the title [Shadow] How should element.requestFullscreen(); work in shadow dom (bugzilla: 27379) Shadow DOM and the Fullscreen API Mar 9, 2016
@annevk
Copy link
Collaborator

annevk commented Mar 9, 2016

Maybe we need shadowRoot.fullscreenElement to point to the inner one. For document.currentScript we decided it would return null (not return the host). But I guess for fullscreenElement the host might be more useful. Hmm.

@rniwa @foolip @upsuper?

@upsuper
Copy link

upsuper commented Mar 10, 2016

Yeah, @annevk's proposal makes sense to me.

@foolip
Copy link
Member

foolip commented Mar 10, 2016

shadowRoot.fullscreenElement makes sense to me too. Would there be any difference between open and closed shadow trees?

FWIW, fullscreen inside a shadow tree doesn't currently work correctly in Chrome, because it still has the :-webkit-full-screen-ancestor selector that's needed to make the ancestor "get out of the way", so to say. And there's an ancestor traversal stops at the shadow boundary, so not all (composed) ancestors match this selector. That'll be fixed by https://bugs.chromium.org/p/chromium/issues/detail?id=370459 hopefully.

@TakayoshiKochi
Copy link
Member

Node.shadowRoot will return null for closed shadow, but if you retain the reference from the return value of attachShadow({mode: 'closed'}), theClosedShadowRoot.fullscreenElement should be accessible.

@annevk
Copy link
Collaborator

annevk commented Mar 10, 2016

There should be no difference between open and closed since there's no reason here to penetrate the boundary.

@foolip
Copy link
Member

foolip commented Mar 10, 2016

Great, nice and simple!

@hayatoito
Copy link
Contributor Author

Let's merge this into #192

@annevk
Copy link
Collaborator

annevk commented Jun 1, 2016

I'd prefer to keep this separate since it affects a different standard. Hope that's okay.

@annevk annevk reopened this Jun 1, 2016
@hayatoito
Copy link
Contributor Author

Ops. I did not realize that fullscreen APIs is here: https://fullscreen.spec.whatwg.org/
Re-opening makes sense.

@TakayoshiKochi
Copy link
Member

Let me try to resolve this.

Do we agree on the following? :

  1. Allow an element in a shadow tree to be fullscreen if the element is connected
  2. document.fullscreenEnabled is effective to shadow trees connected to the document
  3. if containing document of a shadow tree is in an <iframe>, its allowfullscreen attribute is effective to shadow trees connected to its document
  4. document.fullscreenElement returns null if fullscreen element is in a shadow tree

or

(4) When an element in a shadow tree becomes fullscreen, document.fullscreenElement points to its shadow host
(5) shadowRoot.fullscreenElement returns the fullscreen element if any

As far as I understand the only contentious point is that Document.fullscreenElement behaves like Document.currentScript (returns null for shadow) or Document.activeElement (returns retargeted element for shadow).

If there is any other concern, please speak up.

@annevk
Copy link
Collaborator

annevk commented Jun 7, 2016

I think document.fullscreenElement should point to the shadow host. shadowRoot.fullscreenElement can return the fullscreen element, or again a host if it's in a different tree.

We also need to define how the pseudo-classes match.

@TakayoshiKochi
Copy link
Member

TakayoshiKochi commented Jun 7, 2016

As exitFullScreen() API is on document, now I think document.fullscreenElement should return something other than null if any element in shadow is full screen, to detect if there is any element in full screen. So document.fullscreenElement at least should not behave like the currentScript way.
I agree annevk's #180 (comment)

For :fullscreen pseudo class, if we honor the current spec sentence:

The :fullscreen pseudo-class must match any element that has its fullscreen flag set.

we have to decide whether shadow host's fullscreen flag is set when its descendant is fullscreen.
I'm not sure if shadowhost:fullscreen { css property... } makes any sense because the host
itself is not rendered unless the host becomes fullscreen. So we don't want to set fullscreen flag
for shadow host in case its descendant is fullscreen, probably.

Another point:
Should fullscreenchanged / fullscreenerror events be dispatched to document, even when an element under shadow tree becomes full screen?

@rniwa
Copy link
Collaborator

rniwa commented Jun 7, 2016

Well, we DO want :fullscreen to match the host if the rule appears outside the shadow tree. Let us consider custom-video element, which allows an element within its shadow tree to be enter full screen. Then we want to allow users of this component to style the element when it's full-screened. For that to work, the user needs to be able to match the host with :fullscreen (so that it can apply style rules only when it's full screen'ed). I mean this is how we'd implement UA media elements if they were implemented via a shadow tree, right?

@hayatoito
Copy link
Contributor Author

hayatoito commented Jun 7, 2016

It sounds that the situation is very similar to activeElement and :focus which we had discussed before. We can re-use the same logic here for the consistency.

Regarding fullscreenchanged events, I think it is okay to be fired on document.

@rniwa
Copy link
Collaborator

rniwa commented Jun 7, 2016

For fullscreenchanged, we could consider also firing at each ancestor ShadowRoot first before firing on Document but I don't think it's strictly necessary for v1. Maybe a nice enhancement to consider in or before v2.

@TakayoshiKochi
Copy link
Member

Thanks for the comments!

So roughly,

  • document.fullscreenElement can point to a shadow host, and its shadowRoot.fullscreenElement points to the fullscreen element (or a shadow host, recursively)
  • :fullscreen matches to those shadow hosts (can be multiple, if shadows are nested multiple times)
  • fullscreenchanged / fullscreenerror events are dispatched to document
  • If a slotted element becomes fullscreen, the slot's shadowRoot.fullscreenElement is null (like concluded in [Shadow] activeElement behavior seems to break encapsulation #358 for activeElement).

Probably we can consider firing those events to ShadowRoot before they are fired to document in v2.
As document should be accessible to those shadow trees, we don't need ShadowRoot.exitFullscreen().

@annevk
Copy link
Collaborator

annevk commented Jun 7, 2016

I think that's a good outcome, thank you. Are you interested in working on the specification patch as well?

@hayatoito
Copy link
Contributor Author

@TakayoshiKochi Is there any update on this issue?

I am now checking the status of all Shadow DOM v1 spec issues.
Regarding this issue, it looks we have a good outcome, and it is likely to have interoperable behavior without any significant concern. If someone still thinks that there is a great risk for interoperability about this issue, please let me know that.

@TakayoshiKochi
Copy link
Member

I'm about to start updates on spec for this (as well as pointerLock for #192).
They are basically following the way what activeElement does, so I'd think
we all agree they are good to go.

TakayoshiKochi added a commit to TakayoshiKochi/fullscreen that referenced this issue Sep 15, 2016
TakayoshiKochi added a commit to TakayoshiKochi/fullscreen that referenced this issue Sep 15, 2016
TakayoshiKochi added a commit to TakayoshiKochi/fullscreen that referenced this issue Sep 15, 2016
TakayoshiKochi added a commit to TakayoshiKochi/fullscreen that referenced this issue Sep 15, 2016
TakayoshiKochi added a commit to TakayoshiKochi/fullscreen that referenced this issue Sep 19, 2016
TakayoshiKochi added a commit to TakayoshiKochi/fullscreen that referenced this issue Sep 20, 2016
TakayoshiKochi added a commit to TakayoshiKochi/fullscreen that referenced this issue Sep 28, 2016
TakayoshiKochi added a commit to TakayoshiKochi/fullscreen that referenced this issue Sep 28, 2016
TakayoshiKochi added a commit to TakayoshiKochi/fullscreen that referenced this issue Sep 28, 2016
TakayoshiKochi added a commit to TakayoshiKochi/fullscreen that referenced this issue Sep 29, 2016
TakayoshiKochi added a commit to TakayoshiKochi/fullscreen that referenced this issue Sep 30, 2016
TakayoshiKochi added a commit to TakayoshiKochi/fullscreen that referenced this issue Sep 30, 2016
TakayoshiKochi added a commit to TakayoshiKochi/fullscreen that referenced this issue Sep 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants