Open Bug 1349264 Opened 4 years ago Updated 2 years ago

DOMContentLoaded Incorrect Order on Session Restore

Categories

(Core :: DOM: Core & HTML, defect, P3)

52 Branch
defect

Tracking

()

UNCONFIRMED

People

(Reporter: jared.deckard, Unassigned)

Details

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.87 Safari/537.36

Steps to reproduce:

https://jsbin.com/qosiweruva

 1. Load a test page in Firefox (latest 52.0.1)
 2. Check one or more check boxes
 3. Quit Firefox (not just the tab)
 4. Start Firefox
 5. Select "History" > "Restore Previous Session"


Actual results:

The DOMContentLoaded event triggers, then change events trigger for checkboxes being restored.

Firefox seems to be triggering the DOMContentLoaded event as soon as the original cached page is loaded.


Expected results:

Only the DOMContentLoaded event should be triggered, no change events.

Restoring the DOM state should occur before the DOMContentLoaded event.
Summary: DOMContentLoaded restore cache → DOMContentLoaded Incorrect Order on Session Restore
I think I see what the underlying issue is now. This is only occurs when the page can't be cached.

When Firefox tries to restore the page from the cache, it is not there, so it fetches a fresh copy, renders, parses, triggers the DOMContentLoaded event, then restores the DOM state.

If Firefox is able to fetch the document from the cache, it applies the DOM state before triggering the DOMContentLoaded event.
Component: Untriaged → DOM
Product: Firefox → Core
After further testing (listening to DOMContentLoaded directly), I was able to reproduce this issue even when the page was cached, so it appears to be a general ordering issue (not necessarily related to the cache as suggested before).
Component: DOM → DOM: Core & HTML
Priority: -- → P2
Hi Jared, 
Thanks for reporting and doing further testing on this bug. After I saw comment #0 description, I have some questions want to ask, 
please don't take it personally:

1) As I know, there is no session restore specification. What is your base line compare for? 

> Only the DOMContentLoaded event should be triggered, no change events.
2) Why you think we should not fire change events in order to restore all the changed data? 

> Restoring the DOM state should occur before the DOMContentLoaded event.
3) Why you think it should keep that sequence?

Thanks,
John
Flags: needinfo?(jared.deckard)
Thank you for feedback John.

> 1) As I know, there is no session restore specification. What is your base line compare for? 

Google Chrome (Mac OS, OpenSUSE, Windows)

 1. Loads the page from the cache
 2. Applies DOM mutations
 3. Triggers the DOMContentLoaded event

Internet Explorer 11 (Windows)

 1. Loads the page from the cache
 2. Applies DOM mutations
 3. Triggers the DOMContentLoaded event

Safari (Mac OS)

 1. Loads the page from the cache
 2. Triggers the DOMContentLoaded event

> 2) Why you think we should not fire change events in order to restore all the changed data? 

I don't have strong feelings about this.

It seems odd to trigger change events while the document is being restored. It feels like it is an artifact of the implementation and not consistent with the high level operation of restoring a document from a stored session.

On the other hand, I can imagine a use case where an application would want these event to properly restore some program state in the javascript. I am OK with these events occurring, I am just expecting them to be logically grouped with "loading DOM content".

The strongest argument I can think of against it is that it seems to fall outside the MDN definition of a change event:

> The change event is fired for <input>, <select>, and <textarea> elements when a change to the element's value is committed by the user. Unlike the input event, the change event is not necessarily fired for each change to an element's value.
> ...
> Depending on the kind of form element being changed and the way the user interacts with the element, the change event fires at a different moment:
> * When the element is activated (by clicking or using the keyboard) for <input type="radio"> and <input type="checkbox">;
> * When the user commits the change explicitly (e.g. by selecting a value from a <select>'s dropdown with a mouse click, by selecting a date from a date picker for <input type="date">, by selecting a file in the file picker for <input type="file">, etc.);
> * When the element loses focus after its value was changed, but not commited (e.g. after editing the value of <textarea> or <input type="text">).

https://developer.mozilla.org/en-US/docs/Web/Events/change

Additional testing:

If I programmatically check a checkbox, then restore the page:

 - No change events are triggered before the restore
 - The checkbox is not restored
 - No change events are triggered after the restore

It seems wrong not to restore the programmatically checked checkbox.

If I programmatically check a checkbox, then manually check a checkbox, then restore the page:

 - Only a change event for the second checkbox is triggered before the restore
 - Both checkboxes are restored
 - Change events are triggered for both checkboxes after the restore

It seems wrong to trigger a change event for a restored checkbox that was initially checked programmatically and was not allowed to generate a change event.

> 3) Why you think it should keep that sequence?

In the context of a restored document, I expect DOM mutations to be considered DOM content that needs to be loaded, not explicit changes from user interaction. Although the implementation is applying stored DOM mutations to a cached document, the DOM content is not truly loaded when the cached document is loaded and parsed. The DOM content is loaded when the DOM matches the stored session.

The use case for this issue is that GitLab is attaching it's change event listeners on the DOMContentLoaded event. These change listeners trigger network requests to store the changes and display them in the page's change log. Every time a Firefox user restores a GitLab session, all the events that occurred in the previous session are replayed and duplicated in the log. It is also possible for a restored session to contain outdated content that overrides the correct content when the session is restored.

The next available document event is the load event, but actual user initiated change events could be missed by waiting that long to setup the change listener.
Flags: needinfo?(jared.deckard)
> Additional testing:
> 
> If I programmatically check a checkbox, then restore the page:
> 
>  - No change events are triggered before the restore
>  - The checkbox is not restored
>  - No change events are triggered after the restore
> 
> It seems wrong not to restore the programmatically checked checkbox.
> 
> If I programmatically check a checkbox, then manually check a checkbox, then
> restore the page:
> 
>  - Only a change event for the second checkbox is triggered before the
> restore
>  - Both checkboxes are restored
>  - Change events are triggered for both checkboxes after the restore
> 
> It seems wrong to trigger a change event for a restored checkbox that was
> initially checked programmatically and was not allowed to generate a change
> event.
> 
Do other browsers also trigger change events while the document is being restored?

> The use case for this issue is that GitLab is attaching it's change event
> listeners on the DOMContentLoaded event. These change listeners trigger
> network requests to store the changes and display them in the page's change
> log. Every time a Firefox user restores a GitLab session, all the events
> that occurred in the previous session are replayed and duplicated in the
> log. It is also possible for a restored session to contain outdated content
> that overrides the correct content when the session is restored.
> 
Same question as above.
Flags: needinfo?(jared.deckard)
> Do other browsers also trigger change events while the document is being restored?

No. I tested Google Chrome and Internet Explorer 11. Neither trigger change events during a restore.

Safari also does not, but it does not attempt to restore the page state in the first place.

> [Do other browsers also trigger change events while a GitLab session is being restored?]

I tested the original GitLab issue across the same browsers and was only able to replicate it on Firefox.

https://gitlab.com/gitlab-org/gitlab-ce/issues/29743
Flags: needinfo?(jared.deckard)
Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information
Priority: P2 → P3
I am no longer able to reproduce this issue on the latest version of Firefox. Input events are no longer emitted before the DOMContentLoaded event while the page is being restored.
You need to log in before you can comment on or make changes to this bug.