w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.42k stars 650 forks source link

[cssom-view] CSSOM View Module redefines some MouseEvent attributes #4084

Open lukebjerring opened 5 years ago

lukebjerring commented 5 years ago

MouseEvent already has all of screenX, screenY, clientX, clientY (which are also long type, which cssom-view is defining them as double).

But, they're redefined in the partial interface here: https://drafts.csswg.org/cssom-view/#extensions-to-the-mouseevent-interface

Found while tweaking the idlharness in https://github.com/web-platform-tests/wpt/pull/12231

jdm commented 4 years ago

This prevents automated testing of the MouseEvent interface in idlharness.js for any test that includes cssom-view.idl because it causes validation to fail, and that's very disappointing.

lukebjerring commented 4 years ago

@jdm - not sure that it prevents anything, just shows up as a failure (or several). screen-orientation, media-capabilities and cssom-view reference the cssom-view IDL.

https://wpt.fyi/results/?label=master&label=experimental&aligned&q=%28cssom-view%7Cmedia-capabilities%7Cscreen-orientation%29%20and%20idlharness

Only cssom-view has failures, for the unique members subtest, and they're:

FAIL message: assert_true: member screenX is unique expected true got false

Or are you referring to something else?

jdm commented 4 years ago

I'm referring to the fact that on the cssom-view idlharness test there are only three test entries that mention MouseEvent. This is because the MouseEvent object is disabled, but uncommenting that still does not cause the MouseEvent interface to be checked. Only fixing the validation errors allows idlharness.js to continue running tests against the interface that fails validation.

foolip commented 3 years ago

The double definitions of these attributes is still an issue. @smfr @emilio @garykac @travisleithead as the editors of the two specs in questions, which spec do you think should define this? I started writing a patch for CSSOM View, but noticed that some other attributes are defined in terms of these, for example mouseEvent.x is just an alias for mouseEvent.clientX. That could still be maintained even if the definition of clientX is in UI Events, but it made me question what the best split is. Advice?

Note that by now this isn't an issue for idlharness.js tests in WPT, because those use the IDL from https://www.npmjs.com/package/@webref/idl which has patches applied to fix problems like this.