1198230 - "Harness status: OK" + failing test when running "fetch-event-network-error.https.html" test

Status: RESOLVED FIXED

(Testing :: web-platform-tests, defect)

Description

[Noemí Freire (:noemi)]

Checked with in m-c, ba43a48d3c52 revision.

Test run such as |./mach web-platform-tests _mozilla/service-workers/service-worker/fetch-event-network-error.https.html|

Result:
Summary

Harness status: OK

Found 1 tests
1 Fail
Details
*Result	
**Fail

*Test Name
**Rejecting the fetch event or using preventDefault() causes a network error	assert_equals: expected "PASS" but got "FAIL: prevent-default: expected network error but loaded"

*Message
**@https://web-platform.test:8443/_mozilla/service-workers/service-worker/fetch-event-network-error.https.html:37:11
promise callback*@https://web-platform.test:8443/_mozilla/service-workers/service-worker/fetch-event-network-error.https.html:24:1
Test.prototype.step@https://web-platform.test:8443/resources/testharness.js:1380:20
promise callback*promise_test@https://web-platform.test:8443/resources/testharness.js:526:31
@https://web-platform.test:8443/_mozilla/service-workers/service-worker/fetch-event-network-error.https.html:19:1

*Traces: https://pastebin.mozilla.org/8843830

Comment 1

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Waiting on https://github.com/slightlyoff/ServiceWorker/issues/741.

Comment 2

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: MozReview Request: Bug 1198230 - Respect FetchEvent.preventDefault(). r=jdm

Bug 1198230 - Respect FetchEvent.preventDefault(). r?jdm

Update web-platform-tests expected data

Comment 3

[Josh Matthews [:jdm]]

Comment on attachment 8657271 [details]
MozReview Request: Bug 1198230 - Respect FetchEvent.preventDefault(). r=jdm

https://reviewboard.mozilla.org/r/18349/#review16429

::: xpcom/base/ErrorList.h:337
(Diff revision 1)
> +  ERROR(NS_ERROR_INTERCEPTION_CANCELED,   FAILURE(106)),

nit: match the spacing for FAILURE

Also, why is this important to differentiate?

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

If you're adding a new interception error code, can you please consider if it should be handled here or in another docshell case?

  https://dxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp?case=true&from=nsDocShell.cpp#4967

Right now I think a canceled navigation will have bad UX.  Maybe netReset?

Comment 5

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0c25e32dc51d

Comment 6

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9efa362d4f0e

Added nsDocShell cases.

Comment 7

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Comment on attachment 8657271 [details]
MozReview Request: Bug 1198230 - Respect FetchEvent.preventDefault(). r=jdm

Bug 1198230 - Respect FetchEvent.preventDefault(). r=jdm

Update web-platform-tests expected data

Comment 8

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Comment on attachment 8657271 [details]
MozReview Request: Bug 1198230 - Respect FetchEvent.preventDefault(). r=jdm

Bug 1198230 - Respect FetchEvent.preventDefault(). r=jdm

Update web-platform-tests expected data

Comment 9

[Ben Kelly [:bkelly, not reviewing]]

You also need to update nsContentUtils:

  https://dxr.mozilla.org/mozilla-central/source/dom/base/nsContentUtils.cpp#3445

Comment 10

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Comment on attachment 8657271 [details]
MozReview Request: Bug 1198230 - Respect FetchEvent.preventDefault(). r=jdm

r=jdm on IRC. he forgot to check "Ship It".

Comment 11

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6cbacbd82f5c

with additions for nsContentUtils and localized string.

Comment 12

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Comment on attachment 8657271 [details]
MozReview Request: Bug 1198230 - Respect FetchEvent.preventDefault(). r=jdm

Bug 1198230 - Respect FetchEvent.preventDefault(). r=jdm

Update web-platform-tests expected data

Comment 13

[Nigel Babu [:nigelb]]

jdm isn't a peer or owner of dom/workers. If the Modules page is out of date, can you please update it or have someone who does have review permissions grant review?

Comment 14

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

jdm isn't a worker peer, but this change is only in dom/workers because it is related to Service Workers. jdm is the author of the interception event setup we use for Service Workers, which is what this patch modifies.

Comment 15

[Noemí Freire (:noemi)]

Since the patch has been provided by Nikhil assigning this to him, please feel free to change it as needed. Thanks!

Comment 16

[Nigel Babu [:nigelb]]

Cheers. Could you refresh the patch please? It doesn't apply anymore.

Comment 17

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

(In reply to Nigel Babu [:nigelb] from comment #16)
> Cheers. Could you refresh the patch please? It doesn't apply anymore.

I'm just going to land this with other patches when the tree is open. I could have swore I landed this yesterday, but apparently not :(. Thanks!

Comment 18

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b892adb39601

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b892adb39601

Comment 20

[Noemí Freire (:noemi)]

Hi,

just checked on m-c (19f806034f67 revision) and the tes successfully runs. Thanks for fixing it!.

Summary

Harness status: OK

Found 1 tests
1 Pass
Details
Result	Test Name
Pass	Rejecting the fetch event or using preventDefault() causes a network error