Open Bug 1456479 Opened 7 years ago Updated 2 years ago

Implement Request.isReloadNavigation

Categories

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

enhancement

Tracking

()

People

(Reporter: bkelly, Unassigned)

References

(Depends on 2 open bugs, Blocks 1 open bug)

Details

Attachments

(2 files)

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.
Severity: normal → major
Priority: -- → P3
Assignee: nobody → perry
Status: NEW → ASSIGNED
Priority: P3 → P2
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...
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?
Flags: needinfo?(james)
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.
Flags: needinfo?(james) → needinfo?(bzbarsky)
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.
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.
Flags: needinfo?(bzbarsky)
> 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 :)
(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.
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...
Well, looks like marking the test which triggers the popup as "expected: FAIL" or "disabled:" still causes the overall file to fail.
Assignee: perry → nobody
Status: ASSIGNED → NEW
QA Whiteboard: qa-not-actionable

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.

Severity: major → --

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

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: