1484849 - history.go(0) should reload the current page and not involve the entire session history

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect, P2)

Description

[Perry Jiang [:perry] [no longer employee, use ni?]]

Source: https://searchfox.org/mozilla-central/rev/71ef4447db179639be9eff4471f32a95423962d7/dom/base/nsHistory.cpp#167
Spec: https://html.spec.whatwg.org/multipage/history.html#the-history-interface

Comment 1

[Perry Jiang [:perry] [no longer employee, use ni?]]

Blocks 1456479 because history.go(0) [0] reloads the entire outer window, restarting the WPTs and infinite-looping rather than just reloading the inner iframe.

[0] https://searchfox.org/mozilla-central/rev/71ef4447db179639be9eff4471f32a95423962d7/testing/web-platform/tests/service-workers/service-worker/fetch-event.https.html#526

Comment 2

[Perry Jiang [:perry] [no longer employee, use ni?]]

Blocks bug 1456479 because history.go(0) [0] reloads the entire outer window, restarting the WPTs and infinite-looping rather than just reloading the inner iframe.

[0] https://searchfox.org/mozilla-central/rev/71ef4447db179639be9eff4471f32a95423962d7/testing/web-platform/tests/service-workers/service-worker/fetch-event.https.html#526

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Did you test what other browsers do?
(HTML spec's session history definition isn't web compatible and doesn't match even closely to any browser implementation.)

Comment 4

[Perry Jiang [:perry] [no longer employee, use ni?]]

(In reply to Olli Pettay [:smaug] from comment #3)
> Did you test what other browsers do?
> (HTML spec's session history definition isn't web compatible and doesn't
> match even closely to any browser implementation.)

On macOS, both Chrome and Safari only reload the iframe without reloading the outer window. I have a short fix to account for this case that doesn't seem to break any WPT: https://treeherder.mozilla.org/#/jobs?repo=try&revision=49fc1171659fc8f53ba635cf86efeb04de0ab7cd

Comment 5

[Perry Jiang [:perry] [no longer employee, use ni?]]

Try run with `-u all`: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c96dcfab43a31305090b2081553f9eac4a0c82b5

Comment 6

[Perry Jiang [:perry] [no longer employee, use ni?]]

Given that other browsers have this behavior, the proposed/candidate change didn't break any tests [0], and the spec indicates "When the go(delta) method is invoked, if delta is zero, the user agent must act as if the location.reload() method was called instead" [1], should we go forward with a fix?

[0] https://treeherder.mozilla.org/#/jobs?repo=try&revision=c96dcfab43a31305090b2081553f9eac4a0c82b5
[1] https://html.spec.whatwg.org/multipage/history.html#the-history-interface (need to scroll down a bit)

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Yeah, ok, sounds reasonable. Need to ensure there are some wpt tests for this.

Comment 8

[Perry Jiang [:perry] [no longer employee, use ni?]]

Attachment: Bug 1484849 - history.go(0) should reload the current page and not involve the entire session history r?smaug

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/858b41d7b9ed
history.go(0) should reload the current page and not involve the entire session history r=smaug

Comment 10

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/858b41d7b9ed

Comment 11

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/13480 for changes under testing/web-platform/tests

Comment 12

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Can't merge web-platform-tests PR due to failing upstream checks:
Github PR https://github.com/web-platform-tests/wpt/pull/13480
* continuous-integration/travis-ci/pr (https://travis-ci.org/web-platform-tests/wpt/builds/440463121?utm_source=github_status&utm_medium=notification)

Comment 13

[Andrew Sutherland [:asuth] (he/him)]

Seems to have failed the stability check.
ERROR:/home/travis/build/web-platform-tests/wpt/tools/ci:| `/html/browsers/history/the-history-interface/iframe_history_go_0.html` | `iframe's history.go(0) performs a location.reload()` | **TIMEOUT: 5/10, MISSING: 5/10** | `Test timed out` |

Comment 14

[Andrew Sutherland [:asuth] (he/him)]

I re-triggered the travis CI job because there was an error that made it look like a test infra failure due to a marionette related race not having something clobbered into a global at the right time:
JavaScript error: tools/wptrunner/wptrunner/executors/executormarionette.py, line 70: TypeError: window.win.timeout is not a function

The run succeeded.  So now the question is whether the bot will notice or we should hit the merge button ourselves...

Comment 15

[Andrew Sutherland [:asuth] (he/him)]

I hit the merge button at https://github.com/web-platform-tests/wpt/pull/13480 and filed bug 1498655 on the marionette/wptrunner failure.

Comment 16

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged

Comment 17

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged

Comment 18

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged