ServiceWorkerMessageEvent isn't consistent with MessageEvent

[smaug----]

Because of whatwg/html#1882, the interfaces have now different behavior when handling .ports for example.

But I'd recommend just merge the interfaces.

[smaug----]

#988 is also related

[cdumez]

This also impacts ExtendableMessageEvent:
https://w3c.github.io/ServiceWorker/#extendablemessage-event-section

MessageEvent's ports attribute is no longer nullable in the HTML specification. It'd be great to align the similar interfaces in the Service Worker specification (or even merge the interfaces as @smaug---- suggests).

[foolip]

Ping @jungkees (most active committer recently)

[annevk]

@jungkees this is great, but there was also the suggestion to merge MessageEvent and ServiceWorkerMessageEvent. That's still pretty doable I think, no?

[jungkees]

Having looked at the previous discussion around this, we wanted the same for ServiceWorkerMessageEvent at the beginning. Later, for ExtendableMessageEvent, we had a reason not having been able to extend MessageEvent. I think even if we merge ServiceWorkerMessageEvent and MessageEvent at this time, we anyway have to maintain ExtendableMessageEvent interface separately as it's not a MessageEvent. Considering they have been shipped for a while, I'm inclined to regard them as pair of events defined in SW spec.

/cc @jakearchibald

[annevk]

One less class seems good? And MessageEvent / ExtendableMessageEvent we could potentially migrate to a shared mixin at some point.

[jungkees]

I'm not against refactoring. I've been concerned about the impact of the changes to the existing codes. If we do this, implementations should change, but the application codes won't be affected, right?

[annevk]

They could be affected if they use the class in some way. Extend its prototype, typeof it, etc. Seems unlikely, but there's a risk.

[annevk]

To be clear, that's only a problem for ServiceWorkerMessageEvent. The other change would not be noticeable (and implementations wouldn't even have to copy us necessarily there if they didn't care for it).

[foolip]

11 hits for "ServiceWorkerMessageEvent" results from httparchive:har.2016_09_15_chrome_requests_bodies and 6 from httparchive:har.2016_09_15_android_requests_bodies. They're things like http://docs.devdocs.io/dom/index.json?1469400308 and no real usage that I can see.

But, the Blink change would have to happen soon, if the spec changes but Blink waits, we could end up in a very silly place.

[jungkees]

But, the Blink change would have to happen soon, if the spec changes but Blink waits, we could end up in a very silly place.

Indeed. Other browsers also won't be able to completely avoid being in this kind of silly place for the time being I think.

[annevk]

@foolip are you okay with trying/doing it? Should be easy enough for others to follow if so.

[foolip]

I see it's also in Gecko. If one of Blink and Gecko changes right away and the other commits to following before long, I think the risk looks incredibly low, so 👍

[jakearchibald]

I'm happy with this changing. Highly unlikely to break anything, and hopefully we'll see it in beta etc if it does.

[foolip]

So I guess step 1 would be to update MessageEvent in HTML to use (WindowProxy or MessagePort or ServiceWorker)?

[jungkees]

Considering the shared mixin for MessageEvent and ExtendableMessageEvent, wouldn't it be (WindowProxy or MessagePort or ServiceWorker or Client)?

[foolip]

I wasn't thinking about ExtendableMessageEvent, but yeah, I guess it's just as well to fix it all at once. @annevk, do you feel like trying the HTML changes?

[annevk]

For mixins:

• I think I'd rather wait until IDL has actual mixins
• The other issue is what to do with the dictionary, since dictionaries don't support mixins, we'd have to duplicate there for now

Given that, I think we should postpone deduplication there, especially as it doesn't directly impact implementations or developers, but I'll raise an IDL issue on mixins for dictionaries.

For removing ServiceWorkerMessageEvent, I can make the change to HTML to make MessageEvent reusable for that case.

[foolip]

SGTM.

[jungkees]

Sounds good.

[annevk]

Thanks, done.

[LJWatson]

Thanks Remy. Red wine for me... or tequila of course :)

Léonie.

@LeonieWatson tink.uk Carpe diem

On 21/10/2016 13:18, Anne van Kesteren wrote:

Thanks, done.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#989 (comment), or
mute the thread
https://github.com/notifications/unsubscribe-auth/ADVr6f3AQTCAZ9nT6NqylFakryjyGUq9ks5q2K2ygaJpZM4KTie9.

[annevk]

@jungkees I think it would be better if the generated results are somewhat wrong or something is not working well to open a PR and get those issues resolved first. Doing it through comments on a commit is very hard to follow.

[jungkees]

Okay. Got it.

[annevk]

That also makes the review process easier if we start doing more through PRs here. It's been working very well over in WHATWG-land.

[jungkees]

Understood. I'll try to do so. Thanks!