Intent to Implement and Ship: Move pageX/pageY, layerX/layerY to MouseEvent.idl from UIEvent.idl

73 views
Skip to first unread message

Dave Tapuska

unread,
Jun 23, 2015, 12:55:50 PM6/23/15
to blink-dev

Contact emails

dtap...@chromium.org, rby...@chromium.org


Spec

http://www.w3.org/TR/cssom-view/#extensions-to-the-mouseevent-interface


Summary

Currently pageX, pageY, layerX and layerY are defined on the base UIEvent. These fields are only populated on mouse related events and aren't on other UI events such as touch or keyboard events. They make no sense on the non-mouse events.


Motivation

Bring the implementation into spec compliance to be compatible with other vendors.


Microsoft has requested that be standards compliant in this area. See discussion: https://groups.google.com/a/chromium.org/forum/#!topic/input-dev/4p1XZy_17aw


pageX/pageY are used on 16% of page loads while layerX/layerY are used on 2%


Is this feature supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS, Android, and Android WebView)?

Yes.


Compatibility Risk

Low. Since the events always return 0 for the non-mouse events. It is plausible that some sites will read the value but do nothing with it; but sites have already had to contend with them not being present from events generated by IE.


Firefox: Supports these fields on UIEvent.

Safari: Supports these fields on UIEvent.

IE/Edge: Not supported on UIEvent; only on MouseEvent as per specification; and requested a fix by us here.


Bug

http://crbug.com/503274

http://crbug.com/503276


Implementation Review

https://codereview.chromium.org/1201193005




PhistucK

unread,
Jun 23, 2015, 2:20:53 PM6/23/15
to Dave Tapuska, blink-dev
Can you search the index for usage of these properties with non mouse events? Even a small sample.
Code like event.pageX == 0, or event.pageX < 1 or event.pageY - ...scrollTop will give opposite or bad (NaN) results after this change.


☆PhistucK

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

Dave Tapuska

unread,
Jun 23, 2015, 2:48:27 PM6/23/15
to PhistucK, blink-dev
The metrics only report on the UIEvent.prototype; which encompasses MouseEvent and other UIEvents. There isn't a separate metric for the MouseEvent. We could possibly add one; although it isn't clear that this would give us any useful information as code could still be doing something like "if (event.pageX) " and we would track that as a usage. There are other fields on UIEvent that have been tracked (charCode, keyCode, which) in this fashion (wrt KeyboardEvent). The charCode, keyCode, etc. fields are supposed to be only available on KeyboardEvent according to the spec; yet they have a high usage on UIEvent even though they aren't populated at all; so it is likely the "if (defined)" behavior that is increasing the usage metric. The question is do we keep these fields around or not.


PhistucK

unread,
Jun 23, 2015, 2:51:49 PM6/23/15
to Dave Tapuska, blink-dev
Hm... so can you?


☆PhistucK

Rick Byers

unread,
Jun 23, 2015, 3:26:44 PM6/23/15
to Dave Tapuska, blink-dev
Given that IE has always had page* properties on MouseEvent and they've only ever had one reported issue, and there's no legitimate use case for these APIs in any other context, I think the risk of breaking code is very low.

I support making this change now and keeping our eye out for issues.  Trying to add a type-specific use counter before removing this seems like overkill to me (even if the usage came back as non-trivial, my expectation would be that most such code would work just as well without the property existing on UIEvent).

Note that, AFAIK, layerX/layerY are not specified anywhere - so we'd be moving those only for consistency, not for alignment with any spec.

Do we know any history on why Firefox has them on UIEvent like WebKit?  That's surprising to me.

Rick

smaug

unread,
Jun 23, 2015, 4:14:41 PM6/23/15
to Rick Byers, Dave Tapuska, blink-dev
On 06/23/2015 12:26 PM, Rick Byers wrote:
> Given that IE has always had page* properties on MouseEvent and they've only ever had one reported issue
> <https://groups.google.com/a/chromium.org/forum/#!topic/input-dev/4p1XZy_17aw>, and there's no legitimate use case for these APIs in any other
> context, I think the risk of breaking code is very low.
>
> I support making this change now and keeping our eye out for issues. Trying to add a type-specific use counter before removing this seems like
> overkill to me (even if the usage came back as non-trivial, my expectation would be that most such code would work just as well without the property
> existing on UIEvent).
>
> Note that, AFAIK, layerX/layerY are not specified anywhere - so we'd be moving those only for consistency, not for alignment with any spec.
>
> Do we know any history on why Firefox has them on UIEvent like WebKit? That's surprising to me.

That is some really old stuff. Gecko has had those in UIEvent at least since 2001, and I'd assume layerX/Y are originally from Netscape 4.

-Olli


>
> Rick
>
> On Tue, Jun 23, 2015 at 12:55 PM, 'Dave Tapuska' via blink-dev <blin...@chromium.org <mailto:blin...@chromium.org>> wrote:
>
> Contact emails
>
> dtap...@chromium.org <mailto:dtap...@chromium.org>, rby...@chromium.org <mailto:rby...@chromium.org>
>
>
> Spec
>
> http://www.w3.org/TR/cssom-view/#extensions-to-the-mouseevent-interface
>
>
> Summary
>
> Currently pageX, pageY, layerX and layerY are defined on the base UIEvent. These fields are only populated on mouse related events and aren't on
> other UI events such as touch or keyboard events. They make no sense on the non-mouse events.
>
>
> *Motivation*
>
> Bring the implementation into spec compliance to be compatible with other vendors.
>
>
> Microsoft has requested that be standards compliant in this area. See discussion:
> https://groups.google.com/a/chromium.org/forum/#!topic/input-dev/4p1XZy_17aw
>
>
> pageX/pageY are used <https://www.chromestatus.com/metrics/feature/popularity#UIEventPageX> on 16% of page loads while layerX/layerY are used
> <https://www.google.com/url?q=https%3A%2F%2Fwww.chromestatus.com%2Fmetrics%2Ffeature%2Fpopularity%23UIEventLayerX&sa=D&sntz=1&usg=AFQjCNGjYaGXodigTmEZ8LiWDiylqtEW1g> on
> 2%
>
>
> Is this feature supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS, Android, and Android WebView)?
>
> Yes.
>
>
> Compatibility Risk
>
> Low. Since the events always return 0 for the non-mouse events. It is plausible that some sites will read the value but do nothing with it; but
> sites have already had to contend with them not being present from events generated by IE.
>
>
> Firefox: Supports these fields on UIEvent.
>
> Safari: Supports these fields on UIEvent.
>
> IE/Edge: Not supported on UIEvent; only on MouseEvent as per specification; and requested a fix by us here.
>
>
> *Bug*
>
> http://crbug.com/503274
>
> http://crbug.com/503276 <http://crbug.com/503276>
>
>
> *Implementation Review*
>
> https://codereview.chromium.org/1201193005
>
> *
> *
>
>
>
>
> To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org
> <mailto:blink-dev+...@chromium.org>.

Chris Harrelson

unread,
Jun 25, 2015, 12:48:09 PM6/25/15
to smaug, Rick Byers, Dave Tapuska, blink-dev
LGTM


To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

Philip Jägenstedt

unread,
Jun 25, 2015, 4:38:03 PM6/25/15
to Chris Harrelson, smaug, Rick Byers, Dave Tapuska, blink-dev
LGTM2

I share Rick's hunch that waiting for more use counters won't actually tell us whether or not this is safe. Dave mentioned UIEvent.charCode/keyCode, which are shadowed on KeyboardEvent so that these counters are only ever hit when these return 0, and yet usage is incredibly high:

Node.namespaceURI/localName are in the same situation, but with lower usage:

In neither case do I think the use counters are a good estimate of actual risk. In this kind of situation, making the change shortly after a branch point and keeping a lookout for regressions on the long way to master is reasonable, I think.

Rick Byers

unread,
Jun 25, 2015, 4:42:37 PM6/25/15
to Philip Jägenstedt, Chris Harrelson, smaug, Dave Tapuska, blink-dev
On Thu, Jun 25, 2015 at 4:37 PM, Philip Jägenstedt <phi...@opera.com> wrote:
LGTM2

I share Rick's hunch that waiting for more use counters won't actually tell us whether or not this is safe. Dave mentioned UIEvent.charCode/keyCode, which are shadowed on KeyboardEvent so that these counters are only ever hit when these return 0, and yet usage is incredibly high:

Node.namespaceURI/localName are in the same situation, but with lower usage:

In neither case do I think the use counters are a good estimate of actual risk. In this kind of situation, making the change shortly after a branch point and keeping a lookout for regressions on the long way to master is reasonable, I think.

We've got 2 weeks now until M45 branch.  Do you think we should wait to land (assuming we get the 3rd LGTM)?  Personally my hunch is that ~2 weeks prior to branch is sufficient in this case, but I'm OK waiting for M46 to get an extra 4 weeks of bake time if there is concern here.

Philip Jägenstedt

unread,
Jun 25, 2015, 4:46:44 PM6/25/15
to Rick Byers, Chris Harrelson, smaug, Dave Tapuska, blink-dev
On Thu, Jun 25, 2015 at 10:42 PM, Rick Byers <rby...@chromium.org> wrote:
On Thu, Jun 25, 2015 at 4:37 PM, Philip Jägenstedt <phi...@opera.com> wrote:
LGTM2

I share Rick's hunch that waiting for more use counters won't actually tell us whether or not this is safe. Dave mentioned UIEvent.charCode/keyCode, which are shadowed on KeyboardEvent so that these counters are only ever hit when these return 0, and yet usage is incredibly high:

Node.namespaceURI/localName are in the same situation, but with lower usage:

In neither case do I think the use counters are a good estimate of actual risk. In this kind of situation, making the change shortly after a branch point and keeping a lookout for regressions on the long way to master is reasonable, I think.

We've got 2 weeks now until M45 branch.  Do you think we should wait to land (assuming we get the 3rd LGTM)?  Personally my hunch is that ~2 weeks prior to branch is sufficient in this case, but I'm OK waiting for M46 to get an extra 4 weeks of bake time if there is concern here.

Isn't the branch point tomorrow? So says http://www.chromium.org/developers/calendar

What I meant to say is that this is a good timing to maximize bake time, but if the branch point is indeed two weeks away I don't think we should delay just for that.

Philip

Rick Byers

unread,
Jun 25, 2015, 5:10:18 PM6/25/15
to Philip Jägenstedt, Chris Harrelson, smaug, Dave Tapuska, blink-dev
On Thu, Jun 25, 2015 at 4:46 PM, Philip Jägenstedt <phi...@opera.com> wrote:
On Thu, Jun 25, 2015 at 10:42 PM, Rick Byers <rby...@chromium.org> wrote:
On Thu, Jun 25, 2015 at 4:37 PM, Philip Jägenstedt <phi...@opera.com> wrote:
LGTM2

I share Rick's hunch that waiting for more use counters won't actually tell us whether or not this is safe. Dave mentioned UIEvent.charCode/keyCode, which are shadowed on KeyboardEvent so that these counters are only ever hit when these return 0, and yet usage is incredibly high:

Node.namespaceURI/localName are in the same situation, but with lower usage:

In neither case do I think the use counters are a good estimate of actual risk. In this kind of situation, making the change shortly after a branch point and keeping a lookout for regressions on the long way to master is reasonable, I think.

We've got 2 weeks now until M45 branch.  Do you think we should wait to land (assuming we get the 3rd LGTM)?  Personally my hunch is that ~2 weeks prior to branch is sufficient in this case, but I'm OK waiting for M46 to get an extra 4 weeks of bake time if there is concern here.

Isn't the branch point tomorrow? So says http://www.chromium.org/developers/calendar

Yeah there's some discussion about moving this (many people on the Chrome team will be on vacation for the next 2 weeks) but apparently the exact branch point between now and then has not been chosen yet.  There should be a post on chromium-dev shortly.  Anyway I do think we should avoid making this change immediately before branch, so let's at least wait to see how this discussion unfolds.

What I meant to say is that this is a good timing to maximize bake time, but if the branch point is indeed two weeks away I don't think we should delay just for that.

Thanks for the clarification.


Philip

Dimitri Glazkov

unread,
Jul 1, 2015, 2:01:32 PM7/1/15
to Rick Byers, Philip Jägenstedt, Chris Harrelson, smaug, Dave Tapuska, blink-dev
LGTM3.

:DG<
Reply all
Reply to author
Forward
0 new messages