1873074 - alert() dialog content is vertically clipped and unreadable, if your window is too skinny and you have traditional (non-overlay) scrollbars

Status: NEW

(Toolkit :: Content Prompts, defect, P3)

Description

[Daniel Holbert [:dholbert]]

STR:

1. Be sure you're using traditional scrollbars (not overlay scrollbars), i.e. "Always show scrollbars" enabled in Firefox preferences (on Linux) or OS preferences (on Windows/Mac)
2. Load attached testcase and follow instructions, i.e.:
3. Shrink your window to be skinnier than the rectangle in the testcase.
4. Click the button to spawn an alert.

ACTUAL RESULTS:
The text in the alert is almost entirely vertically-clipped, despite the fact that we've got plenty of height available.

EXPECTED RESULTS:
No such clipping. The text ("Hello") should be visible.

It seems that we size the dialog assuming that no horizontal-scrollbar will be present, and then when the scrollbar shows up and steals some space, we shrink the area that's available to the dialog-content, and we end up clipping the text as a result.

This is a regression. Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=40a93120bacc5957b5da058bfcab903a2a184adf&tochange=a5e9a84e1d679ca182188d8f6cc1f7310965d44e

Looks like a regression from bug 1822131. Emilio, maybe you could take a look?

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: screenshot of bug

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1822131

Comment 4

[Emilio Cobos Álvarez (:emilio)]

I can't repro this on Windows without changing the window minimum size (I can't make the window small enough).

This happens because we set a frame minimum width that happens to be bigger than the window width here: https://searchfox.org/mozilla-central/rev/9c509b8feb28c1e76ad41e65bf9fd87ef672b00f/toolkit/modules/SubDialog.sys.mjs#513-519

Daniel, what's the behavior before my patch? Is the frame just smaller?

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: screenshot of before(good) vs after(bad)

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

Daniel, what's the behavior before my patch? Is the frame just smaller?

Here's a screenshot, comparing the good vs. bad builds from mozregression. The alerts themselves look to have the same sizes, and the content all seems to be at the same position.

I'll take a look in inspector to see where we're arriving at a size difference.

Comment 6

[Daniel Holbert [:dholbert]]

Hmm, so we have this XUL structure:

<div xmlns="http://www.w3.org/1999/xhtml" id="dialogGrid">
  <div id="titleContainer" class="dialogRow titleContainer"><span class="titleIcon"></span><span id="titleCropper"><span id="titleText">bug1873074.bmoattachments.org</span></span></div>
  <div class="dialogRow" id="infoRow">
[...]
    <div id="infoContainer">
[...]
      <xul:description xmlns:xul="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" id="infoBody" context="contentAreaContextMenu" noinitialfocus="true">Hello</xul:description>
    </div>
  </div>

Before and after the behavior-change, devtools shows these box sizes:

• dialogGrid is 56px tall. It has two children, titleContainer and infoRow, which use up all 56px of this element's height in their margin-boxes. (see their sizes below, which add up like so: 19px + 16px + 5px + 16px = 56px)
• titleContainer is 19px tall, and has margin-bottom of 16px (taken from --grid-padding)
• infoRow is only 5px tall (perhaps having been shrunk), and it also has the same margin-bottom of 16px from --grid-padding.
• Its child infoContainer is also 5px tall (filling its meager height entirely).

The behavior difference is for this^ element's child, which is infoBody. After the regression, that innermost infoBody element is also 5px tall (and clips its text, since it has overflow-y: auto;); whereas before the change, infoBody refuses to shrink and is 17px tall (the height of its text).

That element is only a flex item (i.e. its parent infoContainer is only a flex container) due to this linux-specific rule:
https://searchfox.org/mozilla-central/rev/9c509b8feb28c1e76ad41e65bf9fd87ef672b00f/toolkit/themes/shared/commonDialog.css#47-52

If I untick the CSS for that rule in devtools, then the element stops getting clipped because it stops being a flex item and stops caring about the shrinkability change from bug 1822131.

So I guess that linux-specific bit of commonDialog.css is partly responsible here and explains why you can't repro on Windows. [EDIT: see next comment]

Comment 7

[Daniel Holbert [:dholbert]]

Er, I guess we have the equivalent CSS rule on windows that makes infoContainer a flex container there as well:
https://searchfox.org/mozilla-central/rev/9c509b8feb28c1e76ad41e65bf9fd87ef672b00f/toolkit/themes/shared/commonDialog.css#10-13,48-51

So the Windows-vs-Linux difference isn't quite explained by that css after all.

Comment 8

[Daniel Holbert [:dholbert]]

Thought: maybe we should give infoBody a min-height of 1em or so?

The issue here seems to be that:
(1) #infoBody has overflow-y:auto (which is responsible for the clipping)
(2) #infoBody has a minimum size of 0 (both implicitly due to being a scrollable flex items, and also explicitly via a min-height:0 rule)
(3) #infoBody is shrinkable down to that minimum size of 0 as of bug 1822131 which removed a rule that prevented shrinking.

Note, here's the CSS rule that gives the styles noted in (1) and (2) here:
https://searchfox.org/mozilla-central/rev/9c509b8feb28c1e76ad41e65bf9fd87ef672b00f/toolkit/components/prompts/content/commonDialog.css#36,43,48,52

Comment 9

[Daniel Holbert [:dholbert]]

It looks like this actually regressed twice (and was briefly fixed in between)!

Comment 0 has the more-recent regression. Slightly earlier builds (before the "good" build at the start of that range) are in fact "bad" -- e.g.
Nightly 112.0a1 (2023-03-01) reproduces the bug.

But substantially older builds, e.g. from a year earlier 2022-03-01, do not reproduce the bug. It looks like that's because the dialog doesn't think it needs a horizontal scrollbar in that old build (at least not in this testcase), so there's no overflow.

Comment 10

[Daniel Holbert [:dholbert]]

Attachment: testcase 2, with buttons to generate skinny popup windows with alert() messages

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

This happens because we set a frame minimum width that happens to be bigger than the window width here: https://searchfox.org/mozilla-central/rev/9c509b8feb28c1e76ad41e65bf9fd87ef672b00f/toolkit/modules/SubDialog.sys.mjs#513-519

Note that you can make skinnier windows via window.open() dialogs (which let themselves be much skinnier than full windows).

Comment 11

[Daniel Holbert [:dholbert]]

Attachment: screenshot of testcase 2's 300px-wide-popup, in Nightly 99 vs. Nightly 123

Here's a screenshot of the popup spawned by the first button in testcase 2, comparing Nightly 99.0a1 (2022-03-01) (left) vs. Nightly 123.0a1 (2024-01-05) (right).

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:mtigley, could you have a look please?

For more information, please visit BugBot documentation.

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1822131

Comment 14

[Dianna Smith [:diannaS]]

:emilio NI you as the author of the regressor

Comment 15

[Emilio Cobos Álvarez (:emilio)]

Looks like an s3 to me given the situations in which this happens. Still worth digging into it to find a good fix, but I don't have time for that right now and it doesn't seem this has come up much in practice.

Comment 16

[Daniel Holbert [:dholbert]]

Attachment: screenshot of this affecting Bitwarden add-on

I just ran into a version of this in the Bitwarden password manager add-on.

STR there:

1. Install Bitwarden from https://addons.mozilla.org/en-US/firefox/addon/bitwarden-password-manager/
2. Sign into the add-on.
3. Open the add-on's dropdown panel from the Firefox toolbar, and click the "pop-out" arrow-out-of-a-box button at its top left (to open a standalone window for the panel)
4. Click the "+" icon (at top right of the Bitwarden window) to create a new login.
5. Click the "x" button on the window toolbar to close the window.

Comment 17

[Daniel Holbert [:dholbert]]

Attachment: screenshot of the UI in Bitwarden before the regression

Notably the "pre-regression" version of the popup in the BitWarden UI is also not-great; see screenshot here. The second line of text is visible at least (that's the thing that regressed), but the first line still runs off the right side of the popup window, so some content is still missing. It looks like this:

This page is asking you to confirm that you want to
information you’ve entered may not be saved.

(The word " leave — " is clipped at the right edge of the first line, though you can see it if you scroll to it using the notification's scrollbar.)

Anyway -- just mentioning since that clipping is visible in the screenshots but independent from what this bug is about.