e.preventDefault should block the network request in fetch event

[wycats]

If the fetch handler calls e.preventDefault(), it should behave like normal DOM events and prevent the default behavior. In this case, the default behavior is the default network fetch.

[annevk]

So this results in a network error if nothing is done?

[alecf]

Not a network error - it just means that the object is free to respond later.

i.e. these two things should more or less be equivalent

1. explicitly preventDefault():

   e.preventDefault();
   networkFetch(someURL).then(function(response) { e.respondWith(response); });

2. return a promise through respondWith

   e.respondWith(new Promise(function(resolver) { resolver.resolve(networkFetch(someURL); }));

[alecf]

The only issue with pattern 1 above is that it more or less requires the "event" object to be used after the scope of the event has completed... I think this breaks some basic DOM Event assumptions and I'm reluctant to say we should support it.

I now think only pattern 2 should be allowed, and that preventDefault() should mostly be a no-op.

[annevk]

Yeah, similar to replying to the install event, you should probably always reply with a promise somehow.

[annevk]

If you have two listeners, should the respondWith method on the second listener throw if a promise is already registered?

[annevk]

No they would race.

I think everyone is agreed that what OP suggests is how this should work.

[tabatkins]

Wait, I thought we discussed this on Wednesday, and calling .respondWith with a promise "claims" the response, so the second fetch handler wouldn't run at all.

[annevk]

It wasn't clear to me that respondWith would also invoke stopImmediatePropagation.

[KenjiBaheux]

Was there any conclusion reached in the meantime?

[annevk]

I don't remember anything.

[jakearchibald]

• You have to call event.respondWith() synchronously, it should throw otherwise
• Calling event.preventDefault() without calling event.respondWith() in any invoked listener will result in a network error
• event.respondWith() also invokes event. stopImmediatePropagation()
• Calling event.respondWith() more than once should throw

[annevk]

The current specification seems broken. E.g. "wait to respond flag" should not unset. Why would it be?

[jungkees]

• respondWith(r) step3 waits until r settles; wait to respond flag is unset after that happens
• [[HandleFetch]] step14.2.3.2 waits until wait to respond flag is unset.

[annevk]

At that point respondWith would no longer throw. Bug?

[jungkees]

Will have respondWith(r) throw when it sets the respond-with error flag in step 4 and 5 of the algorithm.

[jungkees]

Addressed that point: f60dfca

[annevk]

I'm not sure what you just did makes sense or addresses my point. My point was that if respondWith can only be invoked once, the current design does not accomplish that.

[jungkees]

FetchEvent's wait to respond flag is initially unset when it's first fired. respondWith(r)'s step1 is checking whether the event's wait to respond flag is already set and throws, if so.

[annevk]

Yes, but you unset it as I pointed out...

[jungkees]

Oh you're right. Will fix it tomorrow. Thanks for spotting.

[jungkees]

Fixed: dfa1536. Thanks.

[KenjiBaheux]

Tracked at crbug.com/410699 for Blink.

[mfalken]

I don't see where this part from Jake's comment is spec'd:
-You have to call event.respondWith() synchronously, it should throw otherwise

[mfalken]

Unless I'm missing something, this bug should be re-opened. The commits seem to just address calling respondWith() multiple times, whereas the original bug is about preventDefault().

Blink plans to implement the points in Jake's comment: #51 (comment)

[jungkees]

The rest of the points made in #51 (comment) have been addressed: f022c3a.

[mfalken]

Thanks! Looks good to me.

[jungkees]

Closing.