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

visibilitychange should fire after pagehide (not before) #67

Closed
sideshowbarker opened this issue Sep 25, 2020 · 1 comment
Closed

visibilitychange should fire after pagehide (not before) #67

sideshowbarker opened this issue Sep 25, 2020 · 1 comment

Comments

@sideshowbarker
Copy link

The current behavior specified for firing of the visibilitychange event results in the event being fired before the pagehide event has fired — but in Gecko and Blink, the behavior as implemented is the opposite; that is, Gecko and Blink fire visibilitychange after the pagehide event has fired (and the WIP patch for implementing behavior for this case in WebKit https://bug-151234-attachments.webkit.org/attachment.cgi?id=409717 is written to follow the same ordering that Gecko and Blink have, rather than following the spec behavior).

So we should update the spec to match what UAs have actually implemented.

@sideshowbarker
Copy link
Author

In discussions with @cdumez, I realized this needs fixing in the HTML spec instead, so raised whatwg/html#5949 over there.

ryanhaddad pushed a commit to WebKit/WebKit that referenced this issue Dec 22, 2020
https://bugs.webkit.org/show_bug.cgi?id=151234
<rdar://problem/23688763>

Reviewed by Ryosuke Niwa.

LayoutTests/imported/w3c:

Import page-visibility WPT tests from upstream.

* resources/import-expectations.json:
* web-platform-tests/html/browsers/the-window-object/apis-for-creating-and-navigating-browsing-contexts-by-name/no_window_open_when_term_nesting_level_nonzero.window-expected.txt:
* web-platform-tests/page-visibility/*: Added.

Source/WebCore:

Fire a visibilitychange during document unload, as per the specification:
- https://www.w3.org/TR/page-visibility/#reacting-to-visibilitychange-changes
- https://html.spec.whatwg.org/multipage/browsing-the-web.html#unloading-document-visibility-change-steps

Note that the specification currently says to fire the visibilitychange event before the pagehide event.
However, Both Chrome and Firefox fire the pagehide event then the visibilitychange event. This change
aligns our behavior with both Chrome and Firefox. The following bug has been filed against the
specification:
- w3c/page-visibility#67

We also fire a visibilitychange event when coming out of the back/forward cache. This makes sense given
that we fire one when the document enters the back/forward cache. This is also Firefox's behavior.
I have verified that the new fast/history/back-forward-cache-visibility-state.html layout test is passing
in Firefox.

Tests: fast/history/back-forward-cache-visibility-state.html
       imported/w3c/web-platform-tests/page-visibility/idlharness.window.html
       imported/w3c/web-platform-tests/page-visibility/iframe-unload.html
       imported/w3c/web-platform-tests/page-visibility/onvisibilitychange.html
       imported/w3c/web-platform-tests/page-visibility/test_attributes_exist.html
       imported/w3c/web-platform-tests/page-visibility/test_child_document.html
       imported/w3c/web-platform-tests/page-visibility/test_default_view.html
       imported/w3c/web-platform-tests/page-visibility/test_read_only.html
       imported/w3c/web-platform-tests/page-visibility/unload-bubbles.html
       imported/w3c/web-platform-tests/page-visibility/unload.html

* dom/Document.cpp:
(WebCore::Document::visibilityState const):
(WebCore::Document::setHiddenDueToDismissal):
* dom/Document.h:
* history/CachedPage.cpp:
(WebCore::firePageShowAndPopStateEvents):
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::dispatchUnloadEvents):

LayoutTests:

Add test coverage for the visibilitychange event and document.visibilitystate when entering
and coming out of the back/forward cache.

* fast/history/back-forward-cache-visibility-state-expected.txt: Added.
* fast/history/back-forward-cache-visibility-state.html: Added.


Canonical link: https://commits.webkit.org/229786@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@267614 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant