Skip to content
This repository has been archived by the owner on Jul 18, 2022. It is now read-only.

Remove event handling, as it's moving to the HTML spec. #73

Closed
wants to merge 4 commits into from
Closed

Remove event handling, as it's moving to the HTML spec. #73

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cc45c40
Remove event handling, as it's moving to the HTML spec.
noamr Oct 5, 2021
9af3913
Add informative note
noamr Oct 5, 2021
544fc76
Remove whitespace
noamr Oct 5, 2021
89e6977
Remove references to onvisibilitychange
noamr Oct 15, 2021
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
81 changes: 5 additions & 76 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,85 +300,14 @@ <h3>
</h3>
<p>
The {{Document/onvisibilitychange}} attribute is an <a>event handler
IDL attribute</a> for the `"visibilitychange"` event type (see
[[[#reacting-to-visibilitychange]]]).
IDL attribute</a> for the `"visibilitychange"` event type.
marcoscaceres marked this conversation as resolved.
Show resolved Hide resolved
noamr marked this conversation as resolved.
Show resolved Hide resolved
</p>

<p class="note">
The `"visibilitychange"` event is invoked from the [[HTML]] spec.
noamr marked this conversation as resolved.
Show resolved Hide resolved
</p>
</section>
</section>
<section>
<h2 id="reacting-to-visibilitychange">
Reacting to `"visibilitychange"` changes
</h2>
<p>
The <a>task source</a> for these <a>tasks</a> is the <a>user
interaction task source</a>.
</p>
<p>
When the user agent determines that the visibility of the {{Document}}
of the <a>top-level browsing context</a> has changed, the user agent
MUST run the following steps:
</p>
<ol class="algorithm">
<li>Let |doc:Document| be the {{Document}} of the <a>top-level browsing
context</a>.
</li>
<li>If |doc| is now [=Document/visible=]:
<ol>
<li>If traversing to a <a>session history entry</a>, run the <a>now
visible algorithm</a> before running the step to fire the
[=Window/pageshow=] event.
</li>
<li>Otherwise, <a>queue a task</a> that runs the <a>now visible
Copy link
Contributor

Choose a reason for hiding this comment

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

The events defined in HTML now seem to be run synchronously, which is not what was previously defined. Is this intentional? Do we know what implementations are doing here?

Copy link
Author

Choose a reason for hiding this comment

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

The HTML spec only defines the event order in the cases of unload/page hide. Those have to be synchronous otherwise they will never be handled

What's not defined is the non standard firing of that event

Copy link
Contributor

Choose a reason for hiding this comment

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

The HTML spec now fires the events directly, rather than queueing a task for them (which is what this spec defined before this PR). It'd be good to have tests that verify that the events indeed fire synchronously.

Can you expand on "Those have to be synchronous otherwise they will never be handled"?

/cc @rakina

Copy link
Author

Choose a reason for hiding this comment

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

‘

  • If traversing to a session history entry, run the now
    visible algorithm before running the step to fire the
    [=Window/pageshow=] event.’

    that’s the equivalent line in the current spec… synchronous

    if the document is being unloaded or suspended into BFcache, which are the cases handled by the HTML spec, when is the handler going to be called? The JS execution context is about to be gone…

    •

    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    I re-read the algorithm, and you're right. For the cases of history traversal and unload, the events fire synchronously. They are firing async in "other cases" (e.g. tab moved to the background, tab switcher, app switcher, etc), but those other cases are not currently well-defined...

    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    https://html.spec.whatwg.org/multipage/webappapis.html#rendering-opportunity seems to use "page is in the background" without defining it. At worst, maybe we can use something like "page switches from background to foreground" to call the right visibility events?

    Copy link
    Author

    Choose a reason for hiding this comment

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

    whatwg/html#7238

    algorithm</a>.
    </li>
    </ol>
    </li>
    <li>Else if |doc| is now not [=Document/visible=], or if the user agent
    is to [=Window/unload=] |doc|:
    <ol>
    <li>If the user agent is to [=Window/unload=] the {{Document}}, run
    the <a>now hidden algorithm</a> during the <a>unloading document
    visibility change steps</a>.
    </li>
    <li>Otherwise, <a>queue a task</a> that runs the <a>now hidden
    algorithm</a>.
    </li>
    </ol>
    </li>
    </ol>
    <p>
    The <dfn data-export="" data-dfn-type="abstract-op">now visible
    algorithm</dfn> runs the following steps synchronously:
    </p>
    <ol class="algorithm">
    <li>Let |doc:Document| be the {{Document}} of the <a>top-level browsing
    context</a>.
    </li>
    <li>
    <a>Fire an event</a> named "`visibilitychange`" that bubbles, isn't
    cancelable, and has no default action, at the |doc|.
    </li>
    <li>Run <dfn data-export="" data-dfn-type="abstract-op">external now
    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    I vaguely remember other specifications relying on this. Worthwhile to dig through past commits to see why we added this.

    Copy link
    Author

    @noamr noamr Oct 6, 2021 •
    edited
    Loading

    Choose a reason for hiding this comment

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

    The comments on this issue say otherwise, but it's possible...
    I think the discussion here went in the direction of using direct calls rather than hooks, I tend to support that - much more straightforward.

    Copy link
    Contributor

    Choose a reason for hiding this comment

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

    Oh, I see this already came up: #73 (comment)
    Not saying we need to keep this, just that we need to give those specs a proper heads up.

    visible algorithm</dfn> if one is defined by another specification.
    </li>
    </ol>
    <p>
    The <dfn data-export="" data-dfn-type="abstract-op">now hidden
    algorithm</dfn> runs the following steps synchronously:
    </p>
    <ol class="algorithm">
    <li>Let |doc:Document| be the {{Document}} of the <a>top-level browsing
    context</a>.
    </li>
    <li>
    <a>Fire an event</a> named "`visibilitychange`" that bubbles, isn't
    cancelable, and has no default action, at the |doc|.
    </li>
    <li>Run <dfn data-export="">external now hidden algorithm</dfn> if one
    is defined by another specification.
    </li>
    </ol>
    </section>
    <section id="privacy-security">
    <h2>
    Privacy and Security
    Expand Down