Closed Bug 1648265 Opened 4 years ago Closed 4 years ago

Unexpected/useless scrollbars on "zmags" interactive brochure

Categories

(Core :: Layout: Scrolling and Overflow, defect)

Product:
Core
Component:
Layout: Scrolling and Overflow
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla79
Tracking Flags:
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- unaffected
firefox77 --- unaffected
firefox78 --- unaffected
firefox79 + fixed

People

(Reporter: dholbert, Assigned: emilio)

References

(Regression)

Details

(Keywords: regression)

STR:

  1. Load https://www.milgard.com/brochure/tuscany (on a platform where scrollbars take up space)

ACTUAL RESULTS:

  • Useless horizontal and vertical scrollbars are present.
  • If you hit F11 twice to fullscreen and un-fullscreen, the scrollbars disappear but the tracks remain.

EXPECTED RESULTS:

  • No such scrollbars (and consistent layout before/after doing a jaunt into fullscreen mode)

This is a recent regression -- Nightlies from a few days ago don't show these scrollbars. I'm tracking down the regression range now. Current partially-bisected range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=9b38f4b9d883b2bc2c4f3d170942546c2d82aacf&tochange=d2d4420f7b52ba45cf88499374ee4db1633d76a9

(Note that most of this page is rendered in a cross-origin iframe, which might be relevant depending on what we end up isolating as the cause of the regression.)

(And it looks like the iframe has dynamically-computed-by-JS width and height attributes which in part determine its size.)

Has Regression Range: --- → yes

Is it safe to assume you are on a HiDPI display? If so, what's your .devicePixelRatio? I suspect I broke that website for you but it probably was broken for other configurations before...

This is all mildly surprising, let's see where the size is coming from... I have a theory though.

We're not very principled about returning truncated vs. rounded sizes in window.inner* and window.screen* APIs and so on...

Flags: needinfo?(dholbert)

[Tracking Requested - why for this release]: Compat regression on HiDPI screens. We should just back out the CSS change if this turns out not to be easy to figure out.

tracking-firefox79: --- → ?

Will S3 for now barring more evidence of a more prevalent compat problem (then S2 if so).

Severity: -- → S3

(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)

Is it safe to assume you are on a HiDPI display? If so, what's your .devicePixelRatio? I suspect I broke that website for you but it probably was broken for other configurations before...

I am indeed on a HiDPI display, with scale:200% set in Ubuntu settings (and so window.devicePixelRatio is 2)

Flags: needinfo?(dholbert)

So here's what's going on:

this.getHTMLViewerSize=function G(M,L){var N={},O=z!==z.top;N.width=M.offsetWidth;N.height=M.offsetHeight;if(L===F.STANDARD&&!O){if(u.useInnerWindowDimensions()){N.width=z.innerWidth;N.height=z.innerHeight
}if(N.height>0&&u.hasURLBar()){if(u.isURLBarSizeSubtractedFromReportedViewportSize()&&N.height!==320){N.height+=60}else{N.height+=1}}if((N.height===692||N.height===691||N.height===668)&&u.isHeight20PixelsTooHighIf692()){N.height-=20;
y.callLater(z.scrollTo,[0,u.hasURLBar()?1:0])}N=o.adjustSize(y,u,z,N)}return N};

So many questions about that snippet of code, but... :-)

But anyhow, my CSS patch makes the toolbar border 1 device pixel, which for Daniel is 0.5 CSS pixels. This means that a bunch of CSS sizes that we round, like offsetWidth / offsetHeight / innerWidth / innerHeight, etc... Suddenly start returning one device pixel too much, which is enough to cause overflow.

For example, I confirmed with Daniel that this causes an scrollbar on his machine:

<!doctype html>
<script>
  document.documentElement.style.height = window.innerHeight + "px";
</script>

To be clear, this was already an issue on this website at different resolutions and zoom levels, and other browsers have similar issues, my patch just made it worse for the 2x case.

So, I'll file a bug to investigate truncating rather than rounding in those APIs, but meanwhile I think we should just make the toolbar border 1 CSS pixel again.

Assignee: nobody → emilio
Flags: needinfo?(emilio)

Will be fixed by back out of the regressing bug, for now.

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.