matatk / landmarks

Allows you to navigate a web page via WAI-ARIA landmarks, using the keyboard or a pop-up menu
http://matatk.agrip.org.uk/landmarks/
MIT License
125 stars 7 forks source link

Highlight drawn incorrectly when body margins are 0 #394

Open carmacleod opened 4 years ago

carmacleod commented 4 years ago

I created a little landmarks test page, which I want to completely fill the browser's height and width without scrolling. When I set CSS margin for body to 0;

To see the difference between correct highlighting and incorrect highlighting, first run the example and use Alt + Shift + N (Windows) to move through the landmarks. Note that the highlights are drawn a little bit off:

image

Now open DevTools (I'm using Chrome, but saw this in Firefox as well) and uncheck the body style for margin: 0;. The body inherits an 8px margin from the browser's style sheet, which causes an annoying vertical scrollbar, but the highlights are ok:

image

matatk commented 4 years ago

Thanks for the helpful debugging @carmacleod. I'll look into it soon. (As it happens, I hadn't recieved notification of this when you reported it, sorry for the laggy reply.)

matatk commented 3 years ago

Hi @carmacleod, this is interesting... I'm sure that when I tried this back in December, it didn't work, in just the way you and your screengrabs describe, but I just tried it, and it seems to be working fine (in all supported browsers). For Chrome and Edge, I tried the browsers' store version as well as the latest dev version (which shouldn't have any relevant changes).

Are you still able to reproduce this?

carmacleod commented 3 years ago

Yes, I can still reproduce it (just tried in Chrome & FF on Win), but not when there's a vertical scrollbar (which is what happens when I open DevTools because I have it docked on the bottom of the window).

matatk commented 3 years ago

tl;dr: I'm working on it. There's a setting in macOS that determines whether scrollbars are visible or not and it seems to be having an effect here. My code may be using the wrong width measurement. I'll need to do a bit more testing, on Windows too, to make sure I'm really fixing it.


Thanks for checking, and pointing out the scrollbar aspect. What I have found is really unexpected and a bit serendipitous. Turns out it was behaving differently on two Macs running the same macOS and Firefox versions.

I eventually realised that on one of the machines I'd set the scrollbars to always be visible, and on the other I hadn't (we happened to've had a discussion about scrollbar visibility at work not long ago, which I think prompted me to change this setting on my work machine). When the scrollbars are set to be always visible, they don't actually get drawn on your test page, so the problem happens (until you open the DevTools, or zoom in, to really make the scrollbar appear). When the scrollbars are set to auto-hide, the problem doesn't occur in the first place (and still doesn't when DevTools is open to force a scrollbar).

Before I'd found all this out, after you confirmed you could still reproduce the issue, I already had a look at the part of the code I imagined was at fault. I might have got my clientWidth mixed up with my innerWidth. Hopefully that'll turn out to be the case, and when I use the correct one it'll adapt to whether the scrollbar is shown or not.

I can understand the browsers hiding the scrollbar, even when I've asked for it to be visible always, in the case where it will never be possible to scroll the page as-is. However, it feels like something is off given that even when the scrollbar is desired-but-hidden it seems to be affecting the layout calculations. But, either two browsers take an educated different view, to which I'd be sensible to defer pending further investigation, or perhaps the scrollbar rendering is out of their hands.

There's still a bit of investigating to do, and then after I've fixed it, which I hope will be very soon, I'll need to test it quite a bit, too.

carmacleod commented 3 years ago

Nice sleuthing! I wonder if this bit about maybe using pageXoffset and pageYoffset instead of scrollX and scrollY is useful? https://developer.mozilla.org/en-US/docs/Web/API/Element/getBoundingClientRect#cross-browser_fallback

(probably not helpful, but thought I'd mention noticing it)

matatk commented 3 years ago

Thanks for your suggestion. As I understand it, the window.page*Offset properties are aliases for the window.scroll* ones, for compatibility, so I don't think that'd change anything in this case.

I have done some further testing and I am quite confused. Still looking into this, but what I've learnt is that the browsers are returning different values from element.getBoundingClientRect() and window.innerWidth() depending on whether macOS 'show/hide scrollbars' setting is on/off.

In Firefox, here are the bounding boxes I get for the "Store" section...

Scrollbars set to be shown only when needed (it's not until DevTools is opened):

{
  "x": 211.39999389648438,
  "y": 144.60000610351562,
  "width": 346.7166748046875,
  "height": 303.58331298828125,
  "top": 144.60000610351562,
  "right": 558.1166687011719,
  "bottom": 448.1833190917969,
  "left": 211.39999389648438
}

Scrollbars set to be shown 'Always' (but it's not until DevTools is opened):

{
  "x": 211.39999389648438,
  "y": 144.60000610351562,
  "width": 339.2166748046875,
  "height": 303.58331298828125,
  "top": 144.60000610351562,
  "right": 550.6166687011719,
  "bottom": 448.1833190917969,
  "left": 211.39999389648438
}

Chrome reports values almost identical (they'd be the same integers).

The two things that strike me as particularly odd:

These things seem so weird that I tried searching BugZilla and Chromium bugs, but I haven't found anything that matches yet.

I have had some issues building the extension in Windows and haven't been able to see what happens there yet.

carmacleod commented 3 years ago

Weird, as you say. It does feel "buggy". Hopefully you can learn more once you get the Windows build going.

carmacleod commented 3 years ago

Not sure if this gist is helpful: On the Width of Scroll Bars on Mac and Windows. (I'm finding it difficult to fully understand, but maybe you will understand it).

matatk commented 3 years ago

Thanks for the link. As I understand it, I don't think we're quite finding the behaviour described there: it says the scroll bar will only appear if the page is tall enough and you’ve toggled the setting. That part is happening, but the functions are returning values as if the scrollbar was visually rendered even when the page isn’t taller than the viewport (when the scroll bar setting is set to show the bars).

Some more findings, having tested it on Windows…

  1. With the “Automatically hide scroll bars in windows” setting either on or off, the behaviour is the same in Edge and Firefox on Windows: the borders and labels are drawn in the offset manner they shouldn’t be.

  2. Regardless of the setting in Windows’ Settings and regardless of the browser in use, no scrollbar is visible when the DevTools are not shown.

  3. In all cases, the width of the “Store” section was reported the same.

  4. In taking a screen grab, I just happened to notice it flip back to being in the correct position. Eventually worked out this was happening when the border is redrawn, and can be reliably triggered by resizing the browser window.


  5. I then tried it again on the Mac and found the same (in Firefox and Chrome at least).

Are you able to reproduce the border apparently snapping back to being in the correct place when you resize the viewport?

carmacleod commented 3 years ago

Are you able to reproduce the border apparently snapping back to being in the correct place when you resize the viewport?

Yes, I can - interesting.

In Chrome, either maximizing or restoring snaps it back. In Firefox, only maximizing snaps it back (not restore). (Also in Firefox, if I close the landmarks sidebar it snaps back, but not when I open the sidebar).

In both Chrome and Firefox, Ctrl+Shift+B hides/shows the "bookmarks bar" at the top of the window. Either hiding or showing snaps the landmark highlight back to the correct position. (i.e. so if I just type Alt+Shift+N, Ctrl+Shift+B then I can see the shifted position and the corrected position) Slightly easier for testing, because it's more lightweight than bringing up devtools. :) (Hmmm, except for the footer. If the footer is highlighted, then I can toggle its highlight from bad to good and back again just by toggling Ctrl+Shift+B in both FF and Chrome. Only mentioning this in case it's helpful.)

matatk commented 3 years ago

Good to know all of the above; thanks for your quick reply.

In order to be sure this is a bug, I've been trying to create a minimal test case. It seems like it may only happen in certain situations, like if the whole page is a grid (flexbox to be tried later), though I haven't been able to reliably reproduce it yet—more soon/later, hopefully :-).

carmacleod commented 3 years ago

Please note that I am by no means a css expert (far from it! 😄 ) and so I could be doing something incorrectly. I just keep making changes until the page looks the way I want it to! 😂

matatk commented 3 years ago

I'm not a CSS expert either. Experimenting does help.

I think I have created a couple of reasonable test cases. Slightly concerned they're not minimal enough, which makes me wonder if I've really articulated things well, but would be interested as to your thoughts. If you are able to reproduce the problems described on the pages I think I'll start filing bugs, and then find out if I'm grokking what's going on :-).

The content of these might change if it turns out they need tweaking.

carmacleod commented 3 years ago

Your examples work for me, i.e. dotted-line box is positioned incorrectly and scrollbar width is reported even when no scrollbar. I was more likely to see the poorly-positioned box in the Grid example when the DevTools were docked on the bottom, and in the Flexbox example when the DevTools were docked on the side.

Good luck! If you file any bugs, please paste a link here, so I can "star" them (or vote or CC or whatever 😄 ).

matatk commented 3 years ago

Here we go...