Closed Bug 1689188 Opened 3 years ago Closed 3 years ago

documentElement.clientWidth and getBoundingClientRect() return incorrect values on certain pages, and to extensions' content scripts, depending on OS scrollbar settings, when/until the viewport is resized

Categories

(Core :: Layout: Flexbox, defect)

Firefox 85
defect

Tracking

()

RESOLVED INVALID

People

(Reporter: mbugzillam, Unassigned)

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:85.0) Gecko/20100101 Firefox/85.0

Steps to reproduce:

Visit one of the test pages.

Both documentElement.clientWidth and Element.getBoundingClientRect() report incorrect values when:

  • The OS is set to always draw scrollbars;
  • the page is styled to be no taller than the viewport, and CSS Grid or Flexbox is being used,
  • and either:
    • you suddenly and significantly change the viewport's height, e.g. by opening DevTools,
    • or code calling these functions is running in a browser extension's content script and the viewport has not yet been resized.

...until the window's resize event is fired (by resizing the browser, or the DevTools).

For normal pages: you have to suddenly and significantly change the height of the viewport, e.g. by opening DevTools, before the problem occurs, and subsequently resizing the viewport fixes it.

For extension content scripts: incorrect values seem to be returned until the viewport is resized, by any means/amount.

Actual results:

On a page that's styled to be the same height as the viewport, with elements positioned and sized with CSS Grid or Flexbox, after a sudden viewport size change (e.g. opening DevTools docked to the bottom of the window)...

  • documentElement.clientWidth reports a change in viewport width after the sudden resize, even though there's no scrollbar. [Actually on rare occasions I've seen what I think is a vertical scrollbar flash into and out of existence, and the change in width seems like the width of a scrollbar.]
  • The reported documentElement.clientWidth snaps back to its value from before the initial sudden resize, even if the viewport is resized vertically (as if the vertical scrollbar had been there).
  • The reported documentElement.clientWidth snaps back to its value from before the initial sudden resize, even if the viewport is only resized gradually.
  • Element.getBoundingClientRect() reports values that are offset/narrower than those that are visually apparent and reported by the DOM inspection tools.
  • The values returned by Element.getBoundingClientRect() snap back to their expected ones even after a subsequent tiny viewport resize (in any direction).

For code running in an extension's content script under the same circumstances: the values returned by documentElement.clientWidth and Element.getBoundingClientRect() don't match those received by script running on the source page, and are as if a scrollbar is being drawn from page load until the viewport is resized (by any amount).

Expected results:

On a page that's styled to be the same height as the viewport, with elements positioned and sized with CSS Grid/Flexbox...

  • Even when opening DevTools (docked to the bottom of the window) I expect documentElement.clientWidth to continue to report the same (visually apparent) viewport width, as no vertical scrollbar is drawn, even if the OS is set to show scrollbars.
  • The reported documentElement.clientWidth should not change at all if the viewport is resized vertically.
  • The reported documentElement.clientWidth should only change gradually with viewport is resized horizontally.
  • Element.getBoundingClientRect() should report values that match the visual position and size (and those reported by the DOM inspector).
  • The values returned by Element.getBoundingClientRect() should change gradually with viewport resizes.

For code running in an extension's content script on a page such as the above...

  • I expect the values returned by documentElement.clientWidth and Element.getBoundingClientRect() to the content script to match those returned to the scripts running on the source page.
  • If no scrollbar is shown, I expect the values to reflect this.
  • I expect a gradual resize of the viewport to produce gradual changes in the values.

Background:

I was mindful of filing this as a bug because all browsers I tested (Firefox, Chrome and Edge) behave almost the same way. However, this does lead to incorrect information being given to code running both on the page and in a content script, and there's no simple and robust way I can think of to work around it.

I have not tested this on Linux.

Thanks to Carolyn MacLeod for noticing the symptoms of the problem in the Landmarks extension [1], which lead me to (hopefully) get somewhere towards the cause.

[1] https://github.com/matatk/landmarks/issues/394

Possibly related bugs:

This may relate to bug 1209426 though I am not sure on that.

It affects Chrome [1], Edge and Safari too.

P.S. Chrome issue report: https://bugs.chromium.org/p/chromium/issues/detail?id=1171408 (the "[1]" next to "Chrome" above was spurious; oops).

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Component: Untriaged → Layout: Flexbox
Product: Firefox → Core

Is there any chance you have a test-case that demonstrates the issue? client{Width,Height} do have an optimization that looks potentially related:

https://searchfox.org/mozilla-central/rev/0dfbe5a699cc6c73cf8c14d1aa10ba10ef3ec8fa/dom/base/Element.cpp#949-969

But getBoundingClientRect() always updates layout so I'm not sure that is a good guess.

Hi Emilio. There are links to some test pages in my submission above—are you able to reproduce the problem on those pages? (You can check the behaviour with browser extensions there, too.)

The code you linked to looks interesting. In my testing I've tried to get as close to the problem as possible, but it could well be there are some smaller separate issues. That code does look like it could be related in some way (I'm unfamiliar with the codebase though).

If there's any more info you need, I'll do my best to provide it.

Attached file testcase from reporter

Oh, sorry, I totally forgot about them by the time I got to the end :-/

I can repro here on Linux, with the attached test-case (which is just yours but with stuff inline). I just had to load the page in a non-fully-wide window, and switch to fullscreen back and forth with F11.

I wonder what might be going on there...

I can try to poke at this when I have some time, mostly out of curiosity, because I don't think I can explain this without a debugger... Calling getBoundingClientRect() after a bit causes the right result to be returned, which is just super-odd...

Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(emilio)

Ok, I think this is invalid. I finally got some time to look at this.

So the issue is that when you shrink the viewport, the id=border element overflows. That causes vertical and horizontal scrollbars to show up, and then clientWidth obviously returns the inner area which doesn't contain scrollbars. So you then shrink border below the scrollbars, and the scrollbars don't show up anymore, but border is too small.

A way to fix it would be to remove the border element before measuring, or something of that sort.

Status: NEW → RESOLVED
Closed: 3 years ago
Flags: needinfo?(emilio)
Resolution: --- → INVALID

Hi Emilio,

Thank you for looking into this. I can confirm that if I make the DevTools small, the first part of the problem doesn't happen any more (because, as you say, the overflow isn't happening).

However, there is still another part to this, which seems like it might still be a bug, and I have a follow-up question about removing and re-adding the border, as that seems like it could cause other problems...

  1. Browser extension content scripts are given the viewport width as if the scrollbar was displayed all the time, when the OS is set to show them, even if the browser isn't showing the scrollbar. If you are able to try this out with the Landmarks extension, using the links in the test pages on my site, you'll find the border it shows—which is drawn with the same approach—is offset before the viewport is resized, when the OS is set to always show scrollbars. As soon as the OS setting is toggled (at least on macOS 10.15) it has a visible effect. I can provide more info and links to the specific code if that helps.

  2. Thanks for your suggested solution. However, if I want the border to always be visible, removing it on every viewport resize, only to re-add it, seems like it will cause flashing. That would be an annoyance at best and, at worst, it could be a potential health issue for photosensitive users. I'd like to avoid it, but not sure how that can be done. I'm going to research it, but if you know, or have any pointers, it would be good to record the solution here.

best regards,

Matthew

How is the viewport size observed via the content script? Via client* as well?

Regarding 2., there's no need for it to cause flashing. We can't paint while script runs, so as long as you add it back before returning, it should work just fine.

E.g., if you change your script to do something like:

'use strict';
function putBorderOverBox() {
	const box = document.getElementById('centre');
	const border = document.getElementById('border');
        border.style.display = "none";
	const bounds = box.getBoundingClientRect();

	for (const key of ['left', 'width']) {
		document.getElementById(key).innerText = bounds[key].toFixed(0);
	}

	document.getElementById('inner-width').innerText = window.innerWidth;
	document.getElementById('client-width').innerText = document.documentElement.clientWidth;

	border.style.top = window.scrollY + bounds.top + 'px';
	border.style.left = window.scrollX + bounds.left + 'px';
	border.style.width = bounds.width + 'px';
	border.style.height = bounds.height + 'px';
        border.style.display = "";
}

window.addEventListener('load', putBorderOverBox);
window.addEventListener('resize', putBorderOverBox);

I'm pretty sure that shouldn't cause flashes.

Thank you for the code—that's promising to hear and I'll try it out. I was wondering if requestAnimationFrame() might provide even more of a guarantee against flashing (I've not used it before), so now I have two possibilities to try, great!

Yes, the extension uses getBoundingClientRect() and doc.documentElement.clientWidth, but seemingly gets a different value/values than the test case's code does, even before a sudden viewport resize. Here is the code that the extension I maintain uses to size and position its "border" element: https://github.com/matatk/landmarks/blob/ba805a92cc457e71dd6076307d125db93250088a/src/code/borderDrawer.js#L180-L189

I'm of course open to this being a problem in my code, and there's one other thing that might affect this: the "border" <div> has box-sizing: border-box set on it too: https://github.com/matatk/landmarks/blob/ba805a92cc457e71dd6076307d125db93250088a/src/code/borderDrawer.js#L140 - however, when I was making my test case, the problem was only observed when the OS was set to always show scrollbars, which is what made me think this was a browser issue.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: