1933568 - truncated cancel button in "Quit Firefox" dialog

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Sören Hentzschel]

Attachment: screenshot

Since the addition of a third button in bug 1451132, the cancel button in the "Quit Firefox" dialog is truncated.

Comment 1

[BugBot [:suhaib / :marco]]

The bug is marked as tracked for firefox135 (nightly). However, the bug still isn't assigned.

:cbellini, could you please find an assignee for this tracked bug? Given that it is a regression and we know the cause, we could also simply backout the regressor. If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Comment 2

[Dão Gottwald [:dao]]

The bot seems a bit hasty. It's barely been a day, so I guess Jamie might still get to this. If not, Jonathan, could you please find another owner?

Comment 3

[jaime [:jabramson]]

I can work on this! :)
Thanks for submitting the bug!

Comment 4

[Mike Conley (:mconley) (:⚙️)]

I noticed this bug getting filed and poked at it briefly yesterday.

To ease debugging Jaime, you can enable the accented pseudolocale from the Browser Toolbox (meatball menu in Browser Toolbox, "Enabled accented locale") so that you can keep using your Nightly build rather than debug on a German language build. I was able to reproduce in that state.

From a glance, part of the problem appears to be that the <browser> hosting the dialog content is being sized here: https://searchfox.org/mozilla-central/rev/234f91a9d3ebef0d514868701cfb022d5f199cb5/toolkit/modules/SubDialog.sys.mjs#550

and it's using the subdocument scrollWidth here: https://searchfox.org/mozilla-central/rev/234f91a9d3ebef0d514868701cfb022d5f199cb5/toolkit/modules/SubDialog.sys.mjs#530-539

and that's what is being set as the <browser> width. The value that I keep getting is 377px, and I'm fairly certain that's wrong... I don't quite know why the scrollWidth isn't taking the width of the button container into account.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

This feels like we're hitting some Layout weirdness, so if an obvious solution doesn't jump out after a bit of poking, we should probably reach out to the Layout team for tips.

Comment 6

[Dão Gottwald [:dao]]

(In reply to Mike Conley (:mconley) (:⚙️) from comment #4)

and that's what is being set as the <browser> width. The value that I keep getting is 377px, and I'm fairly certain that's wrong... I don't quite know why the scrollWidth isn't taking the width of the button container into account.

Does the button container perhaps overflow?

Comment 7

[jaime [:jabramson]]

ok I'm in SubDialog and poking around, but nothing obvious is jumping out yet...
The buttonBox is overflowing from the 377px container. Is it possible that because the buttons are flex items they aren't being used to calculate scrollWidth?
Where's the best place to get support from the Layout team?

Comment 8

[Mike Conley (:mconley) (:⚙️)]

The #layout Matrix channel is where I'd start. We're in this weird situation where we've got a <browser> in the parent process, and that <browser> loads a document, and then a script in the parent takes that loaded document, samples the scrollWidth, and then sets inline width rules on the <browser> to have it properly sized to the document it contains... I think if you bring that problem to them, with clear steps on how to reproduce it, that might be enough to get some pointers. :)

Comment 9

[Emilio Cobos Álvarez [:emilio]]

So things seem to be working correctly (which doesn't mean as-expected, necessarily).

The main issue is that the dialog is bigger than its specified min-width which is min-width: 29em from commonDialog.css, and so it is capped effectively to that, which is why you get 29 * 13 == 377px.

A potential approach here could be something like:

diff --git a/toolkit/components/prompts/content/commonDialog.css b/toolkit/components/prompts/content/commonDialog.css
index 3521af13c6ed7..948d987563223 100644
--- a/toolkit/components/prompts/content/commonDialog.css
+++ b/toolkit/components/prompts/content/commonDialog.css
@@ -3,7 +3,8 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

 :root {
   min-width: 29em;
+  width: min-content;
 }

 dialog[insecureauth] {

Which would match the sizeToContent behavior of regular dialogs better, I think... You could, I think, also swap them, which would end up with the same behavior effectively (so min-width: min-content and width: 29em), which is a bit easier to reason about.

That does fix the issue locally.

Comment 10

[Emilio Cobos Álvarez [:emilio]]

That said, it seems the issue might be that we basically rely on this being measured while constrained. Another approach could be to use getContentSize() like this, but that's a much lower-level API. So maybe width: min-content is the least bad option here.

Comment 11

[Emilio Cobos Álvarez [:emilio]]

Another alternative would be to allow those buttons to wrap...

Comment 12

[jaime [:jabramson]]

Attachment: Bug 1933568 - Size dialog width to content inside r=mconley,emilio

This resolves an issue where dialogs with three buttons and longer text
were getting cut off.

Comment 13

[Hanna Jones [:hjones]]

This kind of seems like a dupe of Bug 1911426 (or at least whatever fix we go with here will likely cover both issues). I know :tgiles has a patch up that would cause the buttons to wrap as Emilio suggested . I feel like this change is probably safer since it's limited to just dialogs rather than all buttons. That being said in the case highlighted in Bug 1911426 it seems like the dialog would get really wide after this patch, which might lead to a weird experience at smaller screen sizes. :emilio and :dao I'm curious what y'all think is the best way to resolve both of these bugs. Maybe it's even more of a UI issue as to whether or not we want buttons to wrap?

Comment 14

[Emilio Cobos Álvarez [:emilio]]

You still need this fix if for some reason the string can't wrap, right? Though it'd be very weird to have an un-wrappable string of more than 29em.

In any case, those patches shouldn't negatively interact with each other. If you allow wrapping, then the min-content size is smaller and the popup doesn't grow.

So this is a correctness fix, and then bug 1911426 would make us wrap and grow vertically (vs. not wrap and grow horizontally).

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8911904bdba7
Size dialog width to content inside r=mconley,emilio,desktop-theme-reviewers

Comment 16

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/8911904bdba7

Comment 17

[Dana Lucaci, Desktop QA]

The issue is verified fixed with Firefox 136.0a1 (20250126212632) and Firefox 135.0b9 (20250124091819) on macOS 14 and macOS 13.