1608541 - Clear cookies and site data dialog missing buttons

Status: VERIFIED FIXED

(Firefox :: Site Identity, defect)

Description

[Asif Youssuff]

Attachment: Screenshot from 2020-01-10 14-59-44.png

Steps to reproduce:

1. Navigate to a site that sets cookies.
2. Open site identity panel and click "Clear Cookies and Site Data..."

What happens:

Dialog appears without visible buttons.

Expected result:

Buttons should be visible without resizing dialog.

15:06.55 INFO: No more inbound revisions, bisection finished.
15:06.55 INFO: Last good revision: 88b25807638231dcf403d8f0b597fc770f1d24a4
15:06.55 INFO: First bad revision: e183cbb4983cfb3aecf97ab18fad916b91f89e7e
15:06.55 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=88b25807638231dcf403d8f0b597fc770f1d24a4&tochange=e183cbb4983cfb3aecf97ab18fad916b91f89e7e

Comment 1

[Asif Youssuff]

[Tracking Requested - why for this release]: Regression in user visible part of UI.

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Hi Kirk, can you please take a look at this Fx73 regression when you get a chance? Thanks!

Comment 3

[Robin Steuber (they/them) [:bytesized]]

Ok, I finally have an opportunity to look at this, so I will be doing so today. Thanks for your patience.

Comment 4

[Robin Steuber (they/them) [:bytesized]]

I am unable to reproduce this in Windows. The screenshot, however, looks to me like it was taken in Linux. Therefore I am currently running a build on a Linux VM to see if I can reproduce the problem there.

Comment 5

[Robin Steuber (they/them) [:bytesized]]

I'm still unable to reproduce this on a local build on my Ubuntu 18.04.3LTS VM. I'm going to try an official Nightly build on a more recent, non-LTS Ubuntu and see if I can reproduce it there.

Comment 6

[Robin Steuber (they/them) [:bytesized]]

I'm afraid I'm also unable to reproduce this on Ubuntu 19.10 with an official Nightly Build.

Asif - Can you give me any more information on your configuration? What operating system, distribution, and version are you using? Are you still seeing this on Nightly? Could you navigate to "about:support" and give me the information shown there so I can better debug this issue?

Comment 7

[Asif Youssuff]

Attachment: about:support

Kirk,

I am using Ubuntu 20.04 with GNOME 3.34.2.  I am still seeing this in Nightly. 

Here's the about:support from a mozregression build (it happens in a clean profile). 

I am also using a Wayland session. I did a little more testing, and you need to be in a Wayland session and in Wayland mode (MOZ_ENABLE_WAYLAND=1).

Comment 8

[Robin Steuber (they/them) [:bytesized]]

Ah, yes. I have installed wayland on my VM, and I can now reproduce the problem. I'll start working on tracking down the issue.

Comment 9

[Robin Steuber (they/them) [:bytesized]]

I'm afraid that some urgent matters have come up that I need to address. This is still on my to-do list. I plan to resume working on this in about two weeks. Sorry for the delay.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

If this is a Wayland-only issue, it's probably not something we need to track anyway. Note that Fx73 RC week is the week after next, however. Martin, is this something you might be interested in looking at in the mean time?

Comment 11

[Martin Stránský [:stransky] (ni? me)]

Emilio, do you think it may be a variant of Bug 1605724?
Thanks.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Somewhat, the icon problem is probably there, but it doesn't seem just fixing the icon size fixes it (or reloading the dialog).

This seems a bit deeper... I took a look and off-hand I don't know how we end up with the size we end up with...

The window has width="500", so we don't go through the regular "intrinsically sized" codepath. Instead we choose a height of 100px initially here and carry on.

Then something somehow fixes it up in x11, and fixes it up with the wrong value in wayland. I'm not sure where those size_allocate / reconfigure events come from. Any idea?

Of course we can also fix it up by using window.sizeToContent() after load, probably around here... But that is still odd.

As I write this, I think I remember where those constraints come from: https://searchfox.org/mozilla-central/rev/2e355fa82aaa87e8424a9927c8136be184eeb6c7/layout/generic/nsContainerFrame.cpp#632

That will read the min-height of the <window> element and turn it into a constraint for the widget. It seems something is off in that math...

Comment 13

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1608541 - Make min-height in "remove site data" dialog apply to the root element again. r=dao

UL windows are sized constrained by the min-width/height properties on the root
element. If there are none we do XUL layout calculations that apparently don't
always produce the results we want.

The change to make <dialog> not a root element broke this in some cases.

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Except the min-height in the CSS was put on a <dialog>, thus the regression range.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a978aa982f8f
Make min-height in "remove site data" dialog apply to the root element again. r=dao

Comment 16

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/a978aa982f8f

Comment 17

[Ryan VanderMeulen [:RyanVM]]

This looks like it'd be a pretty low-risk ridealong for an RC respin or a dot release. Feel free to nominate the patch for release approval if you agree.

Comment 18

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9124309 [details]
Bug 1608541 - Make min-height in "remove site data" dialog apply to the root element again. r=dao

Beta/Release Uplift Approval Request

• User impact if declined: comment 0
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: comment 0
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): simple CSS fix, scoped to that particular dialog.
• String changes made/needed: none

Comment 19

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

I have reproduced this issue using Firefox 74.0a1 (2010.01.10) on Ubuntu 18.04 also using a Wayland session and in Wayland mode.
I can confirm this issue is fixed, I verified using Firefox 74.0a1 latest nightly build on Ubuntu 18.04 x64 with Wayland session and in Wayland mode.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9124309 [details]
Bug 1608541 - Make min-height in "remove site data" dialog apply to the root element again. r=dao

Minor CSS fix to avoid missing buttons on the remove site data dialog. Approved for 73.0 RC2.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/6a21501eebe8

Comment 22

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

I can confirm this issue is fixed, I verified using Firefox 73.0-build2 on Ubuntu 18.04 x64 with Wayland session and in Wayland mode.