1456479 - Implement Request.isReloadNavigation

Status: NEW

(Core :: DOM: Service Workers, enhancement, P2)

Description

[Ben Kelly [:bkelly, not reviewing]]

The spec is converging on a `Request.isReloadNavigation` flag in:

https://github.com/w3c/ServiceWorker/issues/1167

We should implement this and deprecate FetchEvent.isReload.  Eventually FetchEvent.isReload should be removed in bug 1264175.

Comment 2

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

So there currently are two WPTs failing for reasons that I'd like to believe at not involved with implementing isReloadNavigation:

1) https://searchfox.org/mozilla-central/source/testing/web-platform/tests/service-workers/service-worker/fetch-event.https.html#532 is failing when reloading the iframe triggers the confirm repost prompt (i.e. "To display this page, ... must send information that will repeat any action (such as a search or order confirmation) that was performed earlier."). After accepting the prompt, the console shows an unexpected failure, but the in-browser results reports a pass.

2) https://searchfox.org/mozilla-central/source/testing/web-platform/tests/service-workers/service-worker/fetch-event.https.html#557 is failing because the `frame.contentWindow.history.go(0)` incorrectly loads resources/simple.html?ignore (there is a typo in the current `const anotherUrl` assignment that's missing the ?ignore) rather than reloading resources/simple.html?isReloadNavigation . With a small change to session history (https://treeherder.mozilla.org/#/jobs?repo=try&revision=422bb5f7f94369b6a2d7e4ec52379bf13ab60f36) the test passes, but then there's quite a few oranges...

Comment 3

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

I don't think there is any pref (or similar thing) to suppress the confirm repost prompt; what would be the best way to handle this?

Comment 4

[James Graham [:jgraham]]

I'm not sure I have a good suggestion. I wonder if there's a reason that we don't have a pref for this (e.g. we are concerned about abuse) or whether it would be possible to add one for testing. I think this is not the first time we've had tests fail because of this prompt.

Comment 5

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

It's worth noting that existing Gecko-specific tests deal with this by replacing the XPCOM nsIPromptService and/or nsIPromptFactory to not actually prompt and return the desired value, but that's not a mechanism available to web-platform-tests.

Comment 6

[Boris Zbarsky [:bzbarsky]]

How do other browsers handle the prompt thing?

I have no objection to adding a tristate pref (prompt, allow, deny, or a boolean pref with "not set" meaning prompt, I suppose) for testing purposes; it just hasn't come up before, at least not where I saw.

Comment 7

[James Graham [:jgraham]]

> It's worth noting that existing Gecko-specific tests deal with this by replacing the XPCOM nsIPromptService and/or nsIPromptFactory to not actually prompt and return the desired value, but that's not a mechanism available to web-platform-tests.

I mean we could make marionette do that; it already does something similar to handle alerts I think. But it does seem like a pref is the easier option :)

Comment 8

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

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #6)
> How do other browsers handle the prompt thing?

I couldn't find any automated WPT that triggers the prompt (the only one I found was a manual WPT). For this specific test, the other browsers still aren't passing it.

Comment 9

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

Filed bug 1500617 regarding issue 2) in comment 2. For the time being, using a similar hack like https://searchfox.org/mozilla-central/source/testing/web-platform/tests/service-workers/service-worker/fetch-event.https.html#600 below the iframe's 2nd page load seems to get things working as expected...

Comment 10

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

Attachment: Bug 1456479 - implement Request.isReloadNavigation

Comment 11

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

Attachment: Bug 1456479 - add isReloadNavigation to DOM cache

Depends on D9306

Comment 12

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

Well, looks like marking the test which triggers the popup as "expected: FAIL" or "disabled:" still causes the overall file to fail.

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

In the process of migrating remaining bugs to the new severity system, the severity for this bug cannot be automatically determined. Please retriage this bug using the new severity system.

Comment 14

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

This is potentially mooted as no one implements it; discussion in https://github.com/whatwg/fetch/issues/1280 (not yet resolved)