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

Change algorithm of "collect documents to unfullscreen" #72

Merged
merged 3 commits into from
Feb 24, 2017
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 26 additions & 15 deletions fullscreen.bs
Original file line number Diff line number Diff line change
Expand Up @@ -295,18 +295,33 @@ attribute's getter must run these steps:
<li><p>Return null.
</ol>

<p>A <a>document</a> is said to be a <dfn>simple fullscreen document</dfn> if there is exactly one
<a>element</a> in its <a>top layer</a> that has its <a>fullscreen flag</a> set.

<p class=note>A <a>document</a> with two <a>elements</a> in its <a>top layer</a> can be a
<a>simple fullscreen document</a>. For example, in addition to the <a>fullscreen element</a> there
could be an open <{dialog}> element.

<p>To <dfn>collect documents to unfullscreen</dfn> given <var>doc</var>, run these steps:

<ol>
<li><p>If <var>doc</var>'s <a>top layer</a> consists of more than a single <a>element</a> that has
its <a>fullscreen flag</a> set, return the empty set.

<li><p>Let <var>docs</var> be an ordered set consisting of <var>doc</var>.

<li><p>While <var>docs</var>'s last <a for=/>document</a> has a <a>browsing context container</a>
whose <a>node document</a>'s <a>top layer</a> consists of a single <a>element</a> that has its
<a>fullscreen flag</a> set and does not have its <a>iframe fullscreen flag</a> set (if any), append
that <a>node document</a> to <var>docs</var>.
<li>
<p>While true:

<ol>
<li><p>Let <var>lastDoc</var> be <var>docs</var>'s last <a for=/>document</a>.

<li><p>If <var>lastDoc</var> is not a <a>simple fullscreen document</a>, break.

<li><p>Let <var>container</var> be <var>lastDoc</var>'s <a>browsing context container</a>, if
any, and otherwise break.

<li><p>If <var>container</var>'s <a>iframe fullscreen flag</a> is set, break.

<li><p>Append <var>container</var>'s <a>node document</a> to <var>docs</var>.
</ol>

<li><p>Return <var>docs</var>.
</ol>
Expand All @@ -331,7 +346,8 @@ attribute's getter must run these steps:
<a>active document</a>.
<!-- cross-process -->

<li><p>If <var>topLevelDoc</var> is in <var>docs</var>, then set <var>resize</var> to true.
<li><p>If <var>topLevelDoc</var> is in <var>docs</var>, and it is a
<a>simple fullscreen document</a>, then set <var>resize</var> to true.

<li><p>Return <var>promise</var>, and run the remaining steps <a>in parallel</a>.

Expand All @@ -348,15 +364,10 @@ attribute's getter must run these steps:
<var>doc</var>.
<!-- cross-process -->

<li><p>If <var>resize</var> is true and <var>topLevelDoc</var> is not in <var>exitDocs</var>,
<li><p>If <var>resize</var> is true and <var>topLevelDoc</var> is either not in
<var>exitDocs</var>, or not a <a>simple fullscreen document</a>,
<a>fully exit fullscreen</a> <var>topLevelDoc</var>, reject <var>promise</var> with a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious if Gecko actually has this "fully exit fullscreen" step when exiting? In Blink, at least after the (now reverted) attempt to align with the spec, this didn't make sense. This is the topic of #65

I'm fine with this change if it's just incidental of course, and not the central point of this PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What we actually do in this case is that, after the window gets resized, we synchronously reset the whole document tree to non-fullscreen state regardless of whether the current state is identical to the previous state. Somehow it is simliar to this "fully exit fullscreen" step.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what Blink does as well. I think this change is fine, I'll try to bring it closer to implementation in #65 and ask for your review then.

<code>TypeError</code> exception, and terminate these steps.

<li><p>If <var>exitDocs</var> is the empty set, append <var>doc</var> to <var>exitDocs</var>.

<li><p>If <var>exitDocs</var>'s last <a for=/>document</a> has a
<a>browsing context container</a>, append that <a>browsing context container</a>'s
<a>node document</a> to <var>exitDocs</var>.
<!-- cross-process -->

<li><p>Let <var>descendantDocs</var> be an ordered set consisting of <var>doc</var>'s
Expand Down