9 ms·

A BBC navigation bar component broke depending on the external monitor

294 points by ulrischa 2y ago

cryptonector 2y ago
Don't check screenX/screenY. If `event.type == 'click'` then it's a mouse click.
marcellus23 2y ago
> All we had to do was change the isInvokedByMouse to check that screenX and screenY don't equal 0, rather than checking if they are greater than 0. It's obviously extremely unlikely but what if the mouse is actually at 0,0 when the user clicks? I'm not very familiar with JS, is checking for != 0 really the best/only way to do this? EDIT: actually upon going back, I realized I didn't fully process this sentence originally but it seems to address this: > We should probably do further refactoring of the event handler function, since it's complicated by the fact that it also handles keydown events. For now, though, this fix will do just fine.
- nightpool 2y ago
  But they're already checking for event.name == 'click' in the revised code. So why would you want to filter out some legitimate click events?
  - kulor 2y ago
    Apply Chesterton's Fence principle and assume there are (hopefully) comments in the real code around why this has been put in place
  - marcellus23 2y ago
    Maybe browsers will report `click` events that aren't actually created by a pointer device (maybe a screen reader or something?). But that still raises the question of why you would care. It seems to me like if the platform wants to report it as a `click`, your app should treat it as one and not try to get "clever" about it.
    - pornel 2y ago
      For compatibility with the Web content, the `click` event has become a device-independent activation event. Sites can't be expected to listen for events from every kind of device, so instead all devices send `click`s. They care, because focus for keyboard-controlled screen readers sending "click" should behave differently: an element inside the menu should receive focus, even though it's not the element that has been clicked. Otherwise if focus stayed on top-level menu bar, screen reader users would be lost, and had to navigate to menu's content themselves.
      - marcellus23 2y ago
        Interesting. Seems like something that should be exposed more explicitly.
- whstl 2y ago
  It seems that querying the screen position is just a heuristic they came up with to determine the nature of the event. Instinctively I would use instance of MouseEvent for this, but even this feels risky/hacky to me. My question is why they're relying on those heuristics. My guess is that toggleMenu is being used by multiple event handlers. Or maybe there's something else going on that is specific to their codebase. It's hard to judge without knowing the full picture. EDIT: Aha, there's an answer here: https://news.ycombinator.com/item?id=42174177 https://news.ycombinator.com/item?id=42174177
  - alain94040 2y ago
    You can do a search on GitHub and there is quite a bit of code that is using event.screenX > 0. Maybe someone should file some bug reports. Also, stack overflow suggests that exact code to "differentiate between mouse and keyboard triggering onclick": https://stackoverflow.com/questions/7465006/differentiate-between-mouse-and-keyboard-triggering-onclick https://stackoverflow.com/questions/7465006/differentiate-be...
- tomjen3 2y ago
  No its not. You can do media select on if the primary input device is a pointer device (and, further, if it has high accuracy) and then filter on that. I used it to select which layout to show in the past. If you want to listen to input on touch only then you can do that and call preventDefault on the event so that the browser does not then cause a click event. Or you can just save yourself the trouble and write a click handler.
account42 2y ago
Why are websites getting mouse position in screen coordinates in the first place?
- AlwaysRock 2y ago
  Haha. Welcome to the world of analytics. Lots of sites are recording exactly what you are doing on their site including mouse movement at all times.
- jazzyjackson 2y ago
  If you're reacting to click events, you might want to know the coordinates of where you're clicking. I mostly use this for click and drag stuff since you can get the delta between events and update position of the thing being dragged. As for why they're checking for coordinates instead of checking for event.type is beyond me. Still I appreciate the write up, it is a good puzzle and relatable to come across code you didn't write and ask, why is it important that the click coordinate is nonzero? Why can't we just check that event.target is the button we want to activate? Why are we using JavaScript at all when a details/summary tag would do the same job?
  - yarg 2y ago
    Relative coordinates sure, but why would you need the absolute position? I'm with you on the second point - as unlikely as it is for the click to occur at the origin, it's still a legitimate value being abused as an indicator of something that might not actually be true - quite frankly the code was bad to begin with, and it was still bad after the fix.
- DCH3416 2y ago
  Uh. So they can keep track of what the user is doing? Why would you just send a document when you can generate a heat map of where the user is on your website. And then complain about the performance and wonder why it costs so much to run a modern website.
  - deleted 2y ago
    [deleted]
chgs 2y ago
I’m impressed he wrote a blog without having to get authorisation from a dozen layers of bbc management and lawyers.
nightpool 2y ago
Why are you filtering for screen coordinates in the first place? What if the user is using e.g. an alternative input device that doesn't have a screen? The `click` event should be enough indication that the user has tried to activate the menu. Why reinvent the wheel?
- codetrotter 2y ago
  > Why are you filtering for screen coordinates in the first place? FTA: > The isInvokedByMouse was checking whether the click event was invoked by a mouse or touch pointer – rather than a keyboard – by checking if the screenX or screenY coordinates were a positive number. They were trying to detect whether it was keyboard or mouse activation, and whoever wrote it assumed that screen coordinates of mouse events would always be positive.
  - nightpool 2y ago
    Right, but the article doesn't explain why they cared whether it was keyboard or mouse activation. The linked WebKit bug goes into more detail, but it's still lacking an explanation of why alternative, more common and widely deployed strategies (like having a capturing keyup event that triggers earlier in the render loop) wouldn't be a better idea instead. Also, if you really want to determine whether a MouseEvent is "real" or "synthetic", and you don't want to worry about when mouse events are triggered relative to keyboard events in the event loop (although it doesn't seem very hard to keep track of), it seems like you can use the current click count (i.e., event.detail). This works on both Chrome and Safari—it's 1 for mouse clicks, and 0 for keyboard "clicks", but the spec text is also a little contradictory and under-specified: the "click" event handler says that "the attribute value MUST be 1 when the user begins this action and increments by 1 for each click" (https://w3c.github.io/uievents/#event-type-click https://w3c.github.io/uievents/#event-type-click) but it also says "This MUST be a non-negative integer indicating the number of consecutive clicks of a pointing device button within a specific time" (https://w3c.github.io/uievents/#current-click-count https://w3c.github.io/uievents/#current-click-count), and the definition of "pointing device button" seems to exclude synthetic keyboard events (since those are handled separately)
  - 2y ago
duxup 2y ago
Why are screenX and screenY relevant here as far as their code goes? Where they using those values in their code? Very interesting article but I'm missing the step where it would impact their code ...
- t43562 2y ago
  ...because when they are 0 one can infer that the event came from a keypress rather than a mouse. They want to know this.
  - shdon 2y ago
    Then it would make a lot more sense to check event.pointerType == 'mouse' vs event.pointerType == '' (for keyboard)
    - deleted 2y ago
      [deleted]
    - jorams 2y ago
      That does seem quite obviously better. Even when insisting on checking coordinates why use the coordinate system carried by the event that you have the least control over. Why not .pageX/.pageY, which can't trigger the failure case as long as you make sure the element isn't in the far top left.
  - G1N 2y ago
    Based on the other replies here it seems like it's to differentiate taps vs mouse clicks, keyboard events in js don't have a screenX or screenY property (you can run this in your browser console on this HN post to confirm): (() => { const logEvent = event => console.log({ coords: [event.screenX, event.screenY], type: event.type }); const input = document.querySelector("textarea"); // use "keydown" instead of "keypress" to detect all keyboard input instead of just character producing input input.addEventListener("keydown", logEvent); input.addEventListener("click", logEvent); })(); Type in or click on the reply text input and you'll see that the coords array is undefined for all keyboard events. I haven't tried this equivalent on a touch device however, so not sure how it's handled there.
nozzlegear 2y ago
For anyone who didn't click through to the WebKit bug report the author submitted, a WebKit dev asked him to clarify why the BBC finds it beneficial to be able to detect that the event was sent from a keyboard. This is the author's response: > Ironically, I want interoperability on this to help with use cases relating to accessibility. > I work at the BBC and, on our UK website, our navigation bar menu button behaves slightly differently depending on if it is opened with a pointer or keyboard. The click event will always open the menu, but: > - when opening with a pointer, the focus moves to the menu container. > - when opening with a keyboard, there is no animation to open the menu and the focus moves to the first link in the menu. > Often when opening a menu, we don't want a slightly different behaviour around focus and animations depending on if the user 'clicks' with a pointer or keyboard. > The 'click' event is great when creating user experiences for keyboard users because it is device independent. On keyboards, it is only invoked by Space or Enter key presses. If we were to use the keydown event, we would have to check whether only the the Space or Enter keys were pressed. Source: https://bugs.webkit.org/show_bug.cgi?id=281430 https://bugs.webkit.org/show_bug.cgi?id=281430
- Sayrus 2y ago
  While I can understand the author's need for screenX and screenY, the question remains. Why would screenX return the real screenX position instead of the position within the renderer (I don't think that exists?) or the rendered page (layerX and layerY)? The author's need would be met the same with the renderer position and window positions wouldn't be leaked to all visited websites.
- tshaddox 2y ago
  > Often when opening a menu, we don't want a slightly different behaviour around focus and animations depending on if the user 'clicks' with a pointer or keyboard. Is the word “don’t” a mistake which gives the sentence the opposite of the intended meaning?
  - joshtumath 2y ago
    Hello I am the author and that was indeed a mistake. Whoops!
G1N 2y ago
RE this line of code at the bottom of the article: const isInvokedByMouse = event => event.type === 'click' && (event.screenX !== 0 || event.screenY !== 0); Why do you even have to check if screenX and screenY are non-zero (as opposed to just checking typeof event.screenX == "number")? Wouldn't that mean (and this is a wild edge-case) that if someone positioned their browser window so that the menu was in the top left corner (at position 0,0) the event handler would break again? Is this to block synthetic click events like (<div />).click()? Keyboard events don't have a screenX or screenY from what I remember as well.
- whstl 2y ago
  This is just a heuristic to determine if the event is keydown or click. In the article the author says that the issue is that the same function is handling both events, and they will work on refactoring it to something better. The normal approach is just have different functions answering to different events. Or using more precise information about the event [1], instead of a heuristic. [1] A suggestion was made by this poster: https://news.ycombinator.com/item?id=42174436 https://news.ycombinator.com/item?id=42174436
- aetherspawn 2y ago
  Yeah, comparing to zero is still the wrong thing to do, was dissatisfied after reading the article because the problem still isn't really solved.. just swapping one spooky edge case with another. Who knows, they probably broke the menu for keyboard navigation, voice navigation, eye tracking or something like that. This is one of those cases where you could really "make it make sense" by just using something CSS based.
- chrismorgan 2y ago
  > Keyboard events don't have a screenX or screenY from what I remember as well. Remember that this is on 'click' events. The 'click' event type is a bit of a misnomer: it’s arguably more “activate” than “click”, because (depending a little on platform conventions) it also triggers on Space/Enter if the element is focused. But importantly it’s still a 'click' event: so it’s still a PointerEvent, not a KeyboardEvent. Since various of the fields aren’t appropriate, they get zeroed. So, screenX == 0 && screenY == 0 means either that the pointer is at the top left of the screen, or that the event was not generated by a pointer (that is, by a keyboard). Try it out yourself, if you like, by loading a URL like this and activating by keyboard and by mouse and comparing the events. data:text/html,<button onclick=console.log(event)> In reality, if you used such a check more generally, you’d find it wasn’t such a rare edge case: if the page is fullscreen, corner clicking is actually pretty common, and if you have buttons that are supposed to be in the corner, they should certainly activate on exact-corner clicks. (See also Fitt’s law <https://en.wikipedia.org/wiki/Fitts's_law#Implications_for_UI_design:~:text=This%20effect%20can%20be%20exaggerated%20at%20the%20four%20corners%20of%20a%20screen.,theoretically%20infinitely%20big%20button. https://en.wikipedia.org/wiki/Fitts's_law#Implications_for_U...>.) Fortunately, there’s a proper fix: `event.pointerId === -1` indicates non-pointer (viz. keyboard) activation.
watusername 2y ago
This seems like a self-inflicted bug resulted from incorrect heuristics (assumption that positive screenX/Y values represent mouse event), and the investigation was complicated by inadequate tracing/logging. Instead of checking the more appropriate property that other commenters have suggested (pointerType), I'm a bit surprised that the solution given by the author is to patch up the shaky heuristics even more: > We could deduce from our final two clues the solution: we need to check for negative numbers as well as positive numbers when checking the screenX and screenY coordinates.
- joshtumath 2y ago
  Actually that is what we're going to do. I'm hoping to merging in code, soon, that will change it to use pointerId === -1 and then fall back to screenX === 0. At the time this code was originally written four years ago or whenever it was, not all browsers used PointerEvent for click.
Brian_K_White 2y ago
I would not write a blog post advertizing how I filed a bug with WebKit for my own coding 101 error: unsafe assumptions and relying on side effects and heuristics. Actually not just 101, it's basically with all of us at all levels and for life. So they're in good company having made a mistake everyone makes all the time, but it was a mistake on their part not a bug in WebKit, nore even a "interoperability issue" in WebKit or any browser. They say they weren't aware that negative values were possible and that different browsers produce different values. Ok, but neither of those matters. If the function is even allowed to contain or express a negative value (IE right at the lowest basic level, is the returned data type actually a uint, or is it anything else? a regular int? a string?) then negetive values were always a possibility even if you personally never saw one before. Saying "I didn't expect a number below 0" is barely any different from saying "I didn't expect a number above 10000". The discrepency between browsers doesn't matter and isn't the browsers fault that it tripped you up. You just made a standard boring unsafe assumption like every other programmer ever. The entire problem is that you cared about something you don't actually care about. You assumed that there was meaning in the absolute position of the window or the mouse pointer, when there never was, and you don't actually care about those anyway. The absolute position is like the actual internal-only row number in a db. Every row has a unique one, but it's none of your business what it is. There is only meaning in it's position relative to something else like a button, or relative to it's previous position to track movement. Similarly checking for 0,0 and assuming that means keyboard is just another false heuristic that doesn't actually prove any such thing. The specs may or may not promise that the value will be 0,0 in the event of a keyboard initiated click, but no way it says that it can't be 0,0 any other way. Don't de ashamed of this error because it's common, but don't be proud of calling these WebKit or browser interoperability bugs. Do write up and publish the experience though as a warning and lesson to new developers about assumptions and heuristics and relying on side effects that just happen to work on the developers laptop when they tried it once and shipped it. Also "it's for accessibility" doesn't change anything. Trying to be too smart just makes it worse. Actually that's true just generally for everything.
- dpig_ 2y ago
  I appreciate and fully agree with your perspective on this. A thing was treated as if it were some other thing, and acceptably failed to be that other thing.
andy_ppp 2y ago
What is the correct way to check if you have a mouse click rather than a keyboard click? I would be tempted to set a module level flag based on what most recently happened - if there was a "mousedown" event more recently we'll set isKeyboard to false and isMouse to true and vice-versa if "keydown" is pressed. Then we wouldn't need the isInvokedByMouse and isInvokedByKeyboard functions. Is there a better way? Relying on screen coordinates for this is highly dubious and I would argue a hack.
- matijs 2y ago
  `event.detail` [1] is 0 for keyboard “clicks” and 1 for pointer clicks. 1: https://developer.mozilla.org/en-US/docs/Web/API/UIEvent/detail https://developer.mozilla.org/en-US/docs/Web/API/UIEvent/det...
  - andy_ppp 2y ago
    The whole point of the learning from this article is don’t use hacks and make assumptions about what is happening. This is not for that, this is about how many clicks happened. You might get away with it but it’s not good code.
8organicbits 2y ago
Kudos to BBC for investing in accessibility, and unfortunately discovering a nasty bug. As an industry, why haven't we figured out how to make drop downs that consistently open for all users? Is accessibility just that hard? Are there web frameworks/web components BBC should be using that already handle this? I've been wary (as a backend-focused full-stack developer) about tweaking the browsers components. There's so much nuance to how they work and the implementations are battle tested. The idea of creating a custom text box (for example) without doing extensive research of text box behavior across platforms seems ripe for failure. I notice broken copy/paste and dropped characters often enough (on major corporate sites too). Why are text boxes broken in 2024? React feels arrogant to me now. Personally, I've tried to handle this with server-side templates, CSS frameworks like Bulma, minimal JS. It's not viable for sites demanding slick custom branding (vanity?) but my text boxes work and my site doesn't cost a fortune to develop. Is it accessible to BBC standards? I'm not sure.
- spookie 2y ago
  Agreed. But ultimately many issues arise when user agents customize these elements in very dubious ways. It's ok for the most part, but there's a reason behind reset.css files, and I wager a more nuclear approach was used here to circumvent these issues completely. I'm just trying to reason on their decision here.
- deleted 2y ago
  [deleted]
- chownie 2y ago
  > As an industry, why haven't we figured out how to make drop downs that consistently open for all users? Is accessibility just that hard? Are there web frameworks/web components BBC should be using that already handle this? I don't know the answers to all the questions, but "is accessibility just that hard" is a firm, concrete, YES. Here's some real world examples, modals. If you are not a vision impaired user, you can see what's going on when you're presented with a white box containing ui components swimming in a sea of "don't touch this bit" grey. If you're using a screen reader there's no guarantee that you'll receive any of that information. When your screen reader controls tab through the UI elements and you land back at the top of the box, will your particular screen reader report that to you? Will it list the available interactable elements? Will it list them in the same order as the other screen readers? How about on phone? How about on Mac? Will your screen reader and browser report the inputs right, or will it silently allow the user to fall out of the modal and back into the rest of the site? When it comes to accessibility you can't trust that the OS, browser and the screen reader are cooperative, or even that they'll do something sane in the right situation. In 2019 I had to log a bug with VoiceOver + Safari because a negative CSS margin could cause screen readers to read RTL text blocks out of order. Users with unimpaired vision would see "9/10/2019" and on the screen reader you would hear "ten slash nine slash two-thousand-and-nineteen", as a stopgap measure we had to set the text aria-hidden and put in an invisible p tag there with the correctly ordered text so screen readers wouldn't choke on it. All this to say, sometimes when you see some jank code relating to accessibility there really isn't a better way to do it. Even if you dumped everything, turned the codebase upside down and focused on accessibility first you'd see stuff inexplicably break the moment JAWS or VoiceOver updates.
kernal 2y ago
That flickering Ace Attorney GIF was extremely annoying.
Kwpolska 2y ago
Checking for position != 0 still does not fix it. On Windows and Linux, it is possible for a window to span multiple displays. Someone could set things up so that a valid click target for the button ends up in the (0, 0) position.
a1o 2y ago
> I checked the (DOM UI Events) spec to see if that was correct, but there didn't seem to be any specific information about it Story of my life is finding out the details that apparently matter when I am debugging stuff has not been actually written in the spec (any)
c0detrafficker 2y ago
[flagged]
- teatkin 2y ago
  I would say many people who browse Hacker News given the fact that, at the time of writing, this is the 4th ranked article. Moreover, who cares which company the person writing this post happened to be working for when discovering this interesting browser edge case?
owaislone 2y ago
Very interesting but why would a browser report different coordinates depending on the monitor? I'd assume browser would treat the webpage as if it was the entire screen irrespective of which display it was on. Is there a reason for web APIs to have this information at all? Seems like a potential security/information/tracking risk.
ivanjermakov 2y ago
Skill issue? Should've used viewport, not screen coordinates (read with .clientX and .clientY). I don't see why negative values in screen space is a bug. https://developer.mozilla.org/en-US/docs/Web/CSS/CSSOM_view/Coordinate_systems#viewport https://developer.mozilla.org/en-US/docs/Web/CSS/CSSOM_view/...
jccalhoun 2y ago
Am I missing something? Why are things highlighted in blue? And why is " on Chrome and Firefox, the screenX and screenY properties were negative numbers" repeated so often? I had to go back to reread to make sure I didn't somehow skip back a few lines without noticing it.
joshtumath 2y ago
I've just posted another blog post to provide context and answer some questions people had. Like why are we checking screenX === 0 in the first place? Why do we want different behaviour depending on keyboard or mouse inputs? And I've explain how I've refactored it to prevent more mishaps. I hope it's helpful. https://www.joshtumath.uk/posts/2024-11-18-how-i-refactored-the-bbc-navigation-bar-and-a-follow-up-faq/ https://www.joshtumath.uk/posts/2024-11-18-how-i-refactored-...
tonymet 2y ago
Most people applaud the tech skills needed to reproduce this bug (I do too). Some people shiver at the Mount Everest of clumsy JavaScript code needed to provide a usable web experience. Can you believe that every app has a team of people who just maintain the app's code?
- account42 2y ago
  > Some people shiver at the Mount Everest of clumsy JavaScript code needed to provide a usable web experience. You don't need any javascript to provide a usable web experience. In fact, you are more likely to break usability with it.
  - tonymet 2y ago
    In theory yes, in practice no. Which one matters?
hansottowirtz 2y ago
Reminds me of a bug on our website where a bright green image turned to yellow on an external monitor. Turns out it is a color space issue on wide gamut screens [1]. [1] https://issues.chromium.org/issues/41483538 https://issues.chromium.org/issues/41483538
256_ 2y ago
Some people are pretty negative about the code presented here (which the author criticised in a second post[0]), which makes me wonder, as someone who thankfully hasn't programmed anything non-trivial in Javascript in years: Is checking for (0,0) actually bad code, or is it simply making the most of a bad API? Bear in mind the following: > Also, thank you to Patrick H. Lauke from TetraLogical (editor of the Pointer Events Level 2 spec) for his comment on Mastodon that suggested improving the offending code by checking for pointerType in the PointerEvent interface instead of screenX and screenY. The code provided in the second post[0] uses pointerType if available, else falls back to checking for (0,0). Also: > At the time this code was originally written four years ago, not all browsers treated click events as PointerEvents. They used the MouseEvent interface, so pointerId or pointerType wasn't an option. [0]: https://www.joshtumath.uk/posts/2024-11-18-how-i-refactored-the-bbc-navigation-bar-and-a-follow-up-faq/ https://www.joshtumath.uk/posts/2024-11-18-how-i-refactored-...
OhNoNotAgain_99 2y ago
[dead]

294 points by ulrischa 2y ago

Don't check screenX/screenY. If `event.type == 'click'` then it's a mouse click.

> All we had to do was change the isInvokedByMouse to check that screenX and screenY don't equal 0, rather than checking if they are greater than 0. It's obviously extremely unlikely but what if the mouse is actually at 0,0 when the user clicks? I'm not very familiar with JS, is checking for != 0 really the best/only way to do this? EDIT: actually upon going back, I realized I didn't fully process this sentence originally but it seems to address this: > We should probably do further refactoring of the event handler function, since it's complicated by the fact that it also handles keydown events. For now, though, this fix will do just fine.

nightpool 2y ago
But they're already checking for event.name == 'click' in the revised code. So why would you want to filter out some legitimate click events?
- kulor 2y ago
  Apply Chesterton's Fence principle and assume there are (hopefully) comments in the real code around why this has been put in place
- marcellus23 2y ago
  Maybe browsers will report `click` events that aren't actually created by a pointer device (maybe a screen reader or something?). But that still raises the question of why you would care. It seems to me like if the platform wants to report it as a `click`, your app should treat it as one and not try to get "clever" about it.
  - pornel 2y ago
    For compatibility with the Web content, the `click` event has become a device-independent activation event. Sites can't be expected to listen for events from every kind of device, so instead all devices send `click`s. They care, because focus for keyboard-controlled screen readers sending "click" should behave differently: an element inside the menu should receive focus, even though it's not the element that has been clicked. Otherwise if focus stayed on top-level menu bar, screen reader users would be lost, and had to navigate to menu's content themselves.
    - marcellus23 2y ago
      Interesting. Seems like something that should be exposed more explicitly.
whstl 2y ago
It seems that querying the screen position is just a heuristic they came up with to determine the nature of the event. Instinctively I would use instance of MouseEvent for this, but even this feels risky/hacky to me. My question is why they're relying on those heuristics. My guess is that toggleMenu is being used by multiple event handlers. Or maybe there's something else going on that is specific to their codebase. It's hard to judge without knowing the full picture. EDIT: Aha, there's an answer here: https://news.ycombinator.com/item?id=42174177 https://news.ycombinator.com/item?id=42174177
- alain94040 2y ago
  You can do a search on GitHub and there is quite a bit of code that is using event.screenX > 0. Maybe someone should file some bug reports. Also, stack overflow suggests that exact code to "differentiate between mouse and keyboard triggering onclick": https://stackoverflow.com/questions/7465006/differentiate-between-mouse-and-keyboard-triggering-onclick https://stackoverflow.com/questions/7465006/differentiate-be...
tomjen3 2y ago
No its not. You can do media select on if the primary input device is a pointer device (and, further, if it has high accuracy) and then filter on that. I used it to select which layout to show in the past. If you want to listen to input on touch only then you can do that and call preventDefault on the event so that the browser does not then cause a click event. Or you can just save yourself the trouble and write a click handler.

account42 2y ago

Why are websites getting mouse position in screen coordinates in the first place?

AlwaysRock 2y ago
Haha. Welcome to the world of analytics. Lots of sites are recording exactly what you are doing on their site including mouse movement at all times.
jazzyjackson 2y ago
If you're reacting to click events, you might want to know the coordinates of where you're clicking. I mostly use this for click and drag stuff since you can get the delta between events and update position of the thing being dragged. As for why they're checking for coordinates instead of checking for event.type is beyond me. Still I appreciate the write up, it is a good puzzle and relatable to come across code you didn't write and ask, why is it important that the click coordinate is nonzero? Why can't we just check that event.target is the button we want to activate? Why are we using JavaScript at all when a details/summary tag would do the same job?
- yarg 2y ago
  Relative coordinates sure, but why would you need the absolute position? I'm with you on the second point - as unlikely as it is for the click to occur at the origin, it's still a legitimate value being abused as an indicator of something that might not actually be true - quite frankly the code was bad to begin with, and it was still bad after the fix.
DCH3416 2y ago
Uh. So they can keep track of what the user is doing? Why would you just send a document when you can generate a heat map of where the user is on your website. And then complain about the performance and wonder why it costs so much to run a modern website.
- deleted 2y ago
  [deleted]

chgs 2y ago

I’m impressed he wrote a blog without having to get authorisation from a dozen layers of bbc management and lawyers.

nightpool 2y ago

Why are you filtering for screen coordinates in the first place? What if the user is using e.g. an alternative input device that doesn't have a screen? The `click` event should be enough indication that the user has tried to activate the menu. Why reinvent the wheel?

codetrotter 2y ago
> Why are you filtering for screen coordinates in the first place? FTA: > The isInvokedByMouse was checking whether the click event was invoked by a mouse or touch pointer – rather than a keyboard – by checking if the screenX or screenY coordinates were a positive number. They were trying to detect whether it was keyboard or mouse activation, and whoever wrote it assumed that screen coordinates of mouse events would always be positive.
- nightpool 2y ago
  Right, but the article doesn't explain why they cared whether it was keyboard or mouse activation. The linked WebKit bug goes into more detail, but it's still lacking an explanation of why alternative, more common and widely deployed strategies (like having a capturing keyup event that triggers earlier in the render loop) wouldn't be a better idea instead. Also, if you really want to determine whether a MouseEvent is "real" or "synthetic", and you don't want to worry about when mouse events are triggered relative to keyboard events in the event loop (although it doesn't seem very hard to keep track of), it seems like you can use the current click count (i.e., event.detail). This works on both Chrome and Safari—it's 1 for mouse clicks, and 0 for keyboard "clicks", but the spec text is also a little contradictory and under-specified: the "click" event handler says that "the attribute value MUST be 1 when the user begins this action and increments by 1 for each click" (https://w3c.github.io/uievents/#event-type-click https://w3c.github.io/uievents/#event-type-click) but it also says "This MUST be a non-negative integer indicating the number of consecutive clicks of a pointing device button within a specific time" (https://w3c.github.io/uievents/#current-click-count https://w3c.github.io/uievents/#current-click-count), and the definition of "pointing device button" seems to exclude synthetic keyboard events (since those are handled separately)
- 2y ago

duxup 2y ago

Why are screenX and screenY relevant here as far as their code goes? Where they using those values in their code? Very interesting article but I'm missing the step where it would impact their code ...

t43562 2y ago
...because when they are 0 one can infer that the event came from a keypress rather than a mouse. They want to know this.
- shdon 2y ago
  Then it would make a lot more sense to check event.pointerType == 'mouse' vs event.pointerType == '' (for keyboard)
  - deleted 2y ago
    [deleted]
  - jorams 2y ago
    That does seem quite obviously better. Even when insisting on checking coordinates why use the coordinate system carried by the event that you have the least control over. Why not .pageX/.pageY, which can't trigger the failure case as long as you make sure the element isn't in the far top left.
- G1N 2y ago
  Based on the other replies here it seems like it's to differentiate taps vs mouse clicks, keyboard events in js don't have a screenX or screenY property (you can run this in your browser console on this HN post to confirm): (() => { const logEvent = event => console.log({ coords: [event.screenX, event.screenY], type: event.type }); const input = document.querySelector("textarea"); // use "keydown" instead of "keypress" to detect all keyboard input instead of just character producing input input.addEventListener("keydown", logEvent); input.addEventListener("click", logEvent); })(); Type in or click on the reply text input and you'll see that the coords array is undefined for all keyboard events. I haven't tried this equivalent on a touch device however, so not sure how it's handled there.

nozzlegear 2y ago

For anyone who didn't click through to the WebKit bug report the author submitted, a WebKit dev asked him to clarify why the BBC finds it beneficial to be able to detect that the event was sent from a keyboard. This is the author's response: > Ironically, I want interoperability on this to help with use cases relating to accessibility. > I work at the BBC and, on our UK website, our navigation bar menu button behaves slightly differently depending on if it is opened with a pointer or keyboard. The click event will always open the menu, but: > - when opening with a pointer, the focus moves to the menu container. > - when opening with a keyboard, there is no animation to open the menu and the focus moves to the first link in the menu. > Often when opening a menu, we don't want a slightly different behaviour around focus and animations depending on if the user 'clicks' with a pointer or keyboard. > The 'click' event is great when creating user experiences for keyboard users because it is device independent. On keyboards, it is only invoked by Space or Enter key presses. If we were to use the keydown event, we would have to check whether only the the Space or Enter keys were pressed. Source: https://bugs.webkit.org/show_bug.cgi?id=281430 https://bugs.webkit.org/show_bug.cgi?id=281430

Sayrus 2y ago
While I can understand the author's need for screenX and screenY, the question remains. Why would screenX return the real screenX position instead of the position within the renderer (I don't think that exists?) or the rendered page (layerX and layerY)? The author's need would be met the same with the renderer position and window positions wouldn't be leaked to all visited websites.
tshaddox 2y ago
> Often when opening a menu, we don't want a slightly different behaviour around focus and animations depending on if the user 'clicks' with a pointer or keyboard. Is the word “don’t” a mistake which gives the sentence the opposite of the intended meaning?
- joshtumath 2y ago
  Hello I am the author and that was indeed a mistake. Whoops!

G1N 2y ago

RE this line of code at the bottom of the article: const isInvokedByMouse = event => event.type === 'click' && (event.screenX !== 0 || event.screenY !== 0); Why do you even have to check if screenX and screenY are non-zero (as opposed to just checking typeof event.screenX == "number")? Wouldn't that mean (and this is a wild edge-case) that if someone positioned their browser window so that the menu was in the top left corner (at position 0,0) the event handler would break again? Is this to block synthetic click events like (<div />).click()? Keyboard events don't have a screenX or screenY from what I remember as well.

whstl 2y ago
This is just a heuristic to determine if the event is keydown or click. In the article the author says that the issue is that the same function is handling both events, and they will work on refactoring it to something better. The normal approach is just have different functions answering to different events. Or using more precise information about the event [1], instead of a heuristic. [1] A suggestion was made by this poster: https://news.ycombinator.com/item?id=42174436 https://news.ycombinator.com/item?id=42174436
aetherspawn 2y ago
Yeah, comparing to zero is still the wrong thing to do, was dissatisfied after reading the article because the problem still isn't really solved.. just swapping one spooky edge case with another. Who knows, they probably broke the menu for keyboard navigation, voice navigation, eye tracking or something like that. This is one of those cases where you could really "make it make sense" by just using something CSS based.
chrismorgan 2y ago
> Keyboard events don't have a screenX or screenY from what I remember as well. Remember that this is on 'click' events. The 'click' event type is a bit of a misnomer: it’s arguably more “activate” than “click”, because (depending a little on platform conventions) it also triggers on Space/Enter if the element is focused. But importantly it’s still a 'click' event: so it’s still a PointerEvent, not a KeyboardEvent. Since various of the fields aren’t appropriate, they get zeroed. So, screenX == 0 && screenY == 0 means either that the pointer is at the top left of the screen, or that the event was not generated by a pointer (that is, by a keyboard). Try it out yourself, if you like, by loading a URL like this and activating by keyboard and by mouse and comparing the events. data:text/html,<button onclick=console.log(event)> In reality, if you used such a check more generally, you’d find it wasn’t such a rare edge case: if the page is fullscreen, corner clicking is actually pretty common, and if you have buttons that are supposed to be in the corner, they should certainly activate on exact-corner clicks. (See also Fitt’s law <https://en.wikipedia.org/wiki/Fitts's_law#Implications_for_UI_design:~:text=This%20effect%20can%20be%20exaggerated%20at%20the%20four%20corners%20of%20a%20screen.,theoretically%20infinitely%20big%20button. https://en.wikipedia.org/wiki/Fitts's_law#Implications_for_U...>.) Fortunately, there’s a proper fix: `event.pointerId === -1` indicates non-pointer (viz. keyboard) activation.

watusername 2y ago

This seems like a self-inflicted bug resulted from incorrect heuristics (assumption that positive screenX/Y values represent mouse event), and the investigation was complicated by inadequate tracing/logging. Instead of checking the more appropriate property that other commenters have suggested (pointerType), I'm a bit surprised that the solution given by the author is to patch up the shaky heuristics even more: > We could deduce from our final two clues the solution: we need to check for negative numbers as well as positive numbers when checking the screenX and screenY coordinates.

joshtumath 2y ago
Actually that is what we're going to do. I'm hoping to merging in code, soon, that will change it to use pointerId === -1 and then fall back to screenX === 0. At the time this code was originally written four years ago or whenever it was, not all browsers used PointerEvent for click.

Brian_K_White 2y ago

I would not write a blog post advertizing how I filed a bug with WebKit for my own coding 101 error: unsafe assumptions and relying on side effects and heuristics. Actually not just 101, it's basically with all of us at all levels and for life. So they're in good company having made a mistake everyone makes all the time, but it was a mistake on their part not a bug in WebKit, nore even a "interoperability issue" in WebKit or any browser. They say they weren't aware that negative values were possible and that different browsers produce different values. Ok, but neither of those matters. If the function is even allowed to contain or express a negative value (IE right at the lowest basic level, is the returned data type actually a uint, or is it anything else? a regular int? a string?) then negetive values were always a possibility even if you personally never saw one before. Saying "I didn't expect a number below 0" is barely any different from saying "I didn't expect a number above 10000". The discrepency between browsers doesn't matter and isn't the browsers fault that it tripped you up. You just made a standard boring unsafe assumption like every other programmer ever. The entire problem is that you cared about something you don't actually care about. You assumed that there was meaning in the absolute position of the window or the mouse pointer, when there never was, and you don't actually care about those anyway. The absolute position is like the actual internal-only row number in a db. Every row has a unique one, but it's none of your business what it is. There is only meaning in it's position relative to something else like a button, or relative to it's previous position to track movement. Similarly checking for 0,0 and assuming that means keyboard is just another false heuristic that doesn't actually prove any such thing. The specs may or may not promise that the value will be 0,0 in the event of a keyboard initiated click, but no way it says that it can't be 0,0 any other way. Don't de ashamed of this error because it's common, but don't be proud of calling these WebKit or browser interoperability bugs. Do write up and publish the experience though as a warning and lesson to new developers about assumptions and heuristics and relying on side effects that just happen to work on the developers laptop when they tried it once and shipped it. Also "it's for accessibility" doesn't change anything. Trying to be too smart just makes it worse. Actually that's true just generally for everything.

dpig_ 2y ago
I appreciate and fully agree with your perspective on this. A thing was treated as if it were some other thing, and acceptably failed to be that other thing.

andy_ppp 2y ago

What is the correct way to check if you have a mouse click rather than a keyboard click? I would be tempted to set a module level flag based on what most recently happened - if there was a "mousedown" event more recently we'll set isKeyboard to false and isMouse to true and vice-versa if "keydown" is pressed. Then we wouldn't need the isInvokedByMouse and isInvokedByKeyboard functions. Is there a better way? Relying on screen coordinates for this is highly dubious and I would argue a hack.

matijs 2y ago
`event.detail` [1] is 0 for keyboard “clicks” and 1 for pointer clicks. 1: https://developer.mozilla.org/en-US/docs/Web/API/UIEvent/detail https://developer.mozilla.org/en-US/docs/Web/API/UIEvent/det...
- andy_ppp 2y ago
  The whole point of the learning from this article is don’t use hacks and make assumptions about what is happening. This is not for that, this is about how many clicks happened. You might get away with it but it’s not good code.

8organicbits 2y ago

Kudos to BBC for investing in accessibility, and unfortunately discovering a nasty bug. As an industry, why haven't we figured out how to make drop downs that consistently open for all users? Is accessibility just that hard? Are there web frameworks/web components BBC should be using that already handle this? I've been wary (as a backend-focused full-stack developer) about tweaking the browsers components. There's so much nuance to how they work and the implementations are battle tested. The idea of creating a custom text box (for example) without doing extensive research of text box behavior across platforms seems ripe for failure. I notice broken copy/paste and dropped characters often enough (on major corporate sites too). Why are text boxes broken in 2024? React feels arrogant to me now. Personally, I've tried to handle this with server-side templates, CSS frameworks like Bulma, minimal JS. It's not viable for sites demanding slick custom branding (vanity?) but my text boxes work and my site doesn't cost a fortune to develop. Is it accessible to BBC standards? I'm not sure.

spookie 2y ago
Agreed. But ultimately many issues arise when user agents customize these elements in very dubious ways. It's ok for the most part, but there's a reason behind reset.css files, and I wager a more nuclear approach was used here to circumvent these issues completely. I'm just trying to reason on their decision here.
deleted 2y ago
[deleted]
chownie 2y ago
> As an industry, why haven't we figured out how to make drop downs that consistently open for all users? Is accessibility just that hard? Are there web frameworks/web components BBC should be using that already handle this? I don't know the answers to all the questions, but "is accessibility just that hard" is a firm, concrete, YES. Here's some real world examples, modals. If you are not a vision impaired user, you can see what's going on when you're presented with a white box containing ui components swimming in a sea of "don't touch this bit" grey. If you're using a screen reader there's no guarantee that you'll receive any of that information. When your screen reader controls tab through the UI elements and you land back at the top of the box, will your particular screen reader report that to you? Will it list the available interactable elements? Will it list them in the same order as the other screen readers? How about on phone? How about on Mac? Will your screen reader and browser report the inputs right, or will it silently allow the user to fall out of the modal and back into the rest of the site? When it comes to accessibility you can't trust that the OS, browser and the screen reader are cooperative, or even that they'll do something sane in the right situation. In 2019 I had to log a bug with VoiceOver + Safari because a negative CSS margin could cause screen readers to read RTL text blocks out of order. Users with unimpaired vision would see "9/10/2019" and on the screen reader you would hear "ten slash nine slash two-thousand-and-nineteen", as a stopgap measure we had to set the text aria-hidden and put in an invisible p tag there with the correctly ordered text so screen readers wouldn't choke on it. All this to say, sometimes when you see some jank code relating to accessibility there really isn't a better way to do it. Even if you dumped everything, turned the codebase upside down and focused on accessibility first you'd see stuff inexplicably break the moment JAWS or VoiceOver updates.

kernal 2y ago

That flickering Ace Attorney GIF was extremely annoying.

Kwpolska 2y ago

Checking for position != 0 still does not fix it. On Windows and Linux, it is possible for a window to span multiple displays. Someone could set things up so that a valid click target for the button ends up in the (0, 0) position.

a1o 2y ago

> I checked the (DOM UI Events) spec to see if that was correct, but there didn't seem to be any specific information about it Story of my life is finding out the details that apparently matter when I am debugging stuff has not been actually written in the spec (any)

c0detrafficker 2y ago

[flagged]

teatkin 2y ago
I would say many people who browse Hacker News given the fact that, at the time of writing, this is the 4th ranked article. Moreover, who cares which company the person writing this post happened to be working for when discovering this interesting browser edge case?

owaislone 2y ago

Very interesting but why would a browser report different coordinates depending on the monitor? I'd assume browser would treat the webpage as if it was the entire screen irrespective of which display it was on. Is there a reason for web APIs to have this information at all? Seems like a potential security/information/tracking risk.

ivanjermakov 2y ago

Skill issue? Should've used viewport, not screen coordinates (read with .clientX and .clientY). I don't see why negative values in screen space is a bug. https://developer.mozilla.org/en-US/docs/Web/CSS/CSSOM_view/Coordinate_systems#viewport https://developer.mozilla.org/en-US/docs/Web/CSS/CSSOM_view/...

jccalhoun 2y ago

Am I missing something? Why are things highlighted in blue? And why is " on Chrome and Firefox, the screenX and screenY properties were negative numbers" repeated so often? I had to go back to reread to make sure I didn't somehow skip back a few lines without noticing it.

joshtumath 2y ago

I've just posted another blog post to provide context and answer some questions people had. Like why are we checking screenX === 0 in the first place? Why do we want different behaviour depending on keyboard or mouse inputs? And I've explain how I've refactored it to prevent more mishaps. I hope it's helpful. https://www.joshtumath.uk/posts/2024-11-18-how-i-refactored-the-bbc-navigation-bar-and-a-follow-up-faq/ https://www.joshtumath.uk/posts/2024-11-18-how-i-refactored-...

tonymet 2y ago

Most people applaud the tech skills needed to reproduce this bug (I do too). Some people shiver at the Mount Everest of clumsy JavaScript code needed to provide a usable web experience. Can you believe that every app has a team of people who just maintain the app's code?

account42 2y ago
> Some people shiver at the Mount Everest of clumsy JavaScript code needed to provide a usable web experience. You don't need any javascript to provide a usable web experience. In fact, you are more likely to break usability with it.
- tonymet 2y ago
  In theory yes, in practice no. Which one matters?

hansottowirtz 2y ago

Reminds me of a bug on our website where a bright green image turned to yellow on an external monitor. Turns out it is a color space issue on wide gamut screens [1]. [1] https://issues.chromium.org/issues/41483538 https://issues.chromium.org/issues/41483538

256_ 2y ago

Some people are pretty negative about the code presented here (which the author criticised in a second post[0]), which makes me wonder, as someone who thankfully hasn't programmed anything non-trivial in Javascript in years: Is checking for (0,0) actually bad code, or is it simply making the most of a bad API? Bear in mind the following: > Also, thank you to Patrick H. Lauke from TetraLogical (editor of the Pointer Events Level 2 spec) for his comment on Mastodon that suggested improving the offending code by checking for pointerType in the PointerEvent interface instead of screenX and screenY. The code provided in the second post[0] uses pointerType if available, else falls back to checking for (0,0). Also: > At the time this code was originally written four years ago, not all browsers treated click events as PointerEvents. They used the MouseEvent interface, so pointerId or pointerType wasn't an option. [0]: https://www.joshtumath.uk/posts/2024-11-18-how-i-refactored-the-bbc-navigation-bar-and-a-follow-up-faq/ https://www.joshtumath.uk/posts/2024-11-18-how-i-refactored-...

OhNoNotAgain_99 2y ago

[dead]