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

Deny request on invisible document #60

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

upsuper
Copy link
Member

@upsuper upsuper commented Oct 24, 2016

Fixes #58.


Preview | Diff

@annevk
Copy link
Member

annevk commented Oct 24, 2016

I think we should instead invoke https://w3c.github.io/page-visibility/#dfn-steps-to-determine-the-visibility-state directly.

IDL attributes is generally what is exposed to JavaScript, whereas specification algorithms should operate on what is exposed to IDL. You can contrast this with C++, where we'd also generally use some underlying concept.

@annevk
Copy link
Member

annevk commented Oct 24, 2016

Also, I'd like @foolip to sign off on the general idea.

@foolip
Copy link
Member

foolip commented Oct 24, 2016

Can you confirm that the document.hidden attribute actually true false for background windows? I assumed that would require involving the window manager some how, and that as actually implemented it only returns true for background tabs.

@upsuper
Copy link
Member Author

upsuper commented Oct 25, 2016

It returns false for background window, but true for background tab. It seems in Chrome, background tab wouldn't be allowed to enter fullscreen either, how does that work?

We stop background window from entering fullscreen via detecting whether the document has focus, I think. Chrome seems to allow background window to enter fullscreen. Do we want to add the focus check to the element ready check instead?

@foolip
Copy link
Member

foolip commented Oct 25, 2016

Is the example in #58 (comment) the only thing that this is supposed to prevent? In my testing, the newly opened window is not hidden, which I think matches your testing.

Presumably it's easy to also give the new window focus, but is the goal to prevent all new windows from going fullscreen, or just some cases?

@upsuper
Copy link
Member Author

upsuper commented Oct 25, 2016

I think the goal is to prevent a background window from going fullscreen silently, so probably we want to check whether the document or the top browsing context of document currently has focus.

@foolip
Copy link
Member

foolip commented Oct 25, 2016

Can you test if document.activeElement becomes null in non-active windows, and if there are any events fired? Given http://stackoverflow.com/questions/1060008/is-there-a-way-to-detect-if-a-browser-window-is-not-currently-active I wouldn't be surprised if there's no web exposed API to tell the difference.

@foolip
Copy link
Member

foolip commented May 2, 2017

Ping @upsuper

@upsuper
Copy link
Member Author

upsuper commented May 2, 2017

document.activeElement doesn't become null. focus and blur would be fired when switching between windows, but changing the focus to address bar also triggers blur.

If it is not allowed to trigger fullscreen asynchronously, the top level document should always get focused before requestFullscreen is called, unless a document from another window requests that.

So should we allow a foreground document to put a background document into fullscreen? Given that window cannot raise itself in general, I don't see how allowing that could be useful.

@foolip
Copy link
Member

foolip commented May 3, 2017

So should we allow a foreground document to put a background document into fullscreen? Given that window cannot raise itself in general, I don't see how allowing that could be useful.

I agree that doesn't sound useful. What should the spec say to prevent that? One thing that might work is a document-bound user gesture concept, so that it's only possible to request fullscreen for the document that was clicked. Is that what you had in mind?

Base automatically changed from master to main January 15, 2021 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Fullscreen request on invisible document should be denied
3 participants