1873002 - Certificate Manager popup doesn't grow with content

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P3)

Description

[erosman]

Attachment: certificate-manager1a.png

I noticed a little hiccup in the UI.
about:preferences#privacy -> View certificates...
I added a few exceptions and the buttons went off the popup.
I had to close it using the X and then reopen it.

STR

• Go to: about:preferences#privacy -> View certificates...
• Click "Add Exceptions..."
• Add new exceptions
• New server will be added to the display
• Repeat
• The box grows and pushes the buttons out of the picture

Comment 1

[erosman]

Attachment: certificate-manager1a.png

Comment 2

[erosman]

Attachment: certificate-manager2a.png

Comment 3

[erosman]

Attachment: certificate-manager3a.png

Comment 4

[:Gijs (he/him)]

Is this a regression?

Comment 5

[:Gijs (he/him)]

(This was previously fixed in bug 1801607...)

Comment 6

[erosman]

I believe setting an overall max-height (30vh should be good) for the inner server box to scroll could be a good approach to the issue.
The regression may not fix all the issues involved.

Comment 7

[:Gijs (he/him)]

(In reply to erosman from comment #6)

I believe setting an overall max-height (30vh should be good)

That means there'll be vertical dead space if users resize the dialog on tall (rotated) screens, so that doesn't seem great to me.

The list should adapt its size to the size of the dialog and scroll when there are too many items to fit in whatever the available space for the list was.

The regression may not fix all the issues involved.

I don't understand this comment. Do you know if this used to work? When did it break? (I don't know of any servers I can use to test this problem with so can't easily find the regression window; hopefully for you it would be trivial to run mozregression)

Comment 8

[erosman]

Do you know if this used to work?

I have been adding certificates for 10+ years and it used to be fine.

When did it break?

No idea, but I reported the UI issue very long time ago (cant recall if it was here or on matrix). That is why I said regression to the last change might not solve the issue.

Comment 9

[Daniel Bodea [:danibodea]]

I would like to test this issue, investigate whether it is a regression and provide a regression range or close it if it was fixed, however, I need to know what kind of server "locations" I can add as exceptions. Could you help me attempt reproduction? Thanks!

Comment 10

[erosman]

however, I need to know what kind of server "locations" I can add as exceptions. Could you help me attempt reproduction?

The reason I regularly have to add exceptions is that the target domain is blocked at my location, therefore I need to use their IP instead. However, the certificate, although valid, lists the domains and not the IP, therefore a mismatch occurs.

To create a list:

• Make a list of few HTTPS domains
• Get their IPs
• Add the IP in the exception dialogue box

Here is an example:

• https://example.com
• IP: 93.184.216.34
• Go to: about:preferences#privacy -> View certificates...
• Click "Add Exceptions..."
• Enter https://93.184.216.34/
• Click "Get Certificate"
• After it gets the certificate, click "Confirm Security Exception"

Screenshot to follow ...

Comment 11

[erosman]

Attachment: certificate-manager4.png

Example with example.com IP

Comment 12

[Daniel Bodea [:danibodea]]

I have investigated for a regressor and the result is: bug 1822578. It appears to be a regression from branch 113.

More information that Mozregression provided:
Bug 1822578 - Make flex="1" on XUL set a zero flex basis like the flex shorthand does. r=Gijs,mconley,settings-reviewers,desktop-theme-reviewers,dao

In a setup with:

<hbox>
<something flex="1"/>
<something-else/>
</hbox>

Before bug 1822131 <something flex="1"> ended up with flex-basis: auto,
but was the only thing able to shrink, so <something-else> stayed the
same size.

After that bug however <something-else> is able to shrink too, so both
elements shrink. This wouldn't happen if flex="1" actually worked like
flex: 1 does.

However flex: 1 causes stuff like explicit main sizes to be
(effectively) ignored, so we need to fix up a few cases where now we'd
start flexing too much. For that, add a debug assert to
nsFlexContainerFrame to catch the would-be behavior changes here.

For the most part they're actually no-op since they're setting tiny
sizes, but preferences and devtools needed a couple real fixes.

The profile selection spacer is useless (zero-size).

Hopefully the last xul.css change I need to do :')

Differential Revision: https://phabricator.services.mozilla.com/D172704

2024-01-11T15:03:06.354000: DEBUG : Did not find a branch, checking all integration branches
2024-01-11T15:03:06.358000: INFO : The bisection is done.
2024-01-11T15:03:06.360000: INFO : Stopped

Comment 13

[:Gijs (he/him)]

Emilio, are you able to take a look here?

Comment 14

[:Gijs (he/him)]

This would be S2 (breaks functionality, no easy workaround) except I can't imagine many people are using this given it went undiscovered for 8-9 months...

Comment 15

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1873002 - Make the richlistbox contents not grow the richlistbox. r=Gijs

This matches the behavior of XUL trees, and also what we did for the
languages listboxes and bug 1860214.

This is enough to fix the bug, but more in-depth fix incoming too.

Comment 16

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1873002 - Make dialog contents shrinkable even when not scrollable. r=Gijs

This is preferable behavior. Otherwise the behavior changes with
and without scrolling enabled, which is rather weird.

Depends on D198395

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d2f0aa747888
Make the richlistbox contents not grow the richlistbox. r=Gijs

Comment 18

[Cristian Tuns]

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

Comment 19

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

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox122 to wontfix.

For more information, please visit BugBot documentation.

Comment 20

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9372501 [details]
Bug 1873002 - Make the richlistbox contents not grow the richlistbox. r=Gijs

Beta/Release Uplift Approval Request

• User impact if declined: comment 0
• Is this code covered by automated tests?: Unknown
• 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): Trivial CSS fix
• String changes made/needed: none
• Is Android affected?: No

Comment 21

[Donal Meehan [:dmeehan]]

Comment on attachment 9372501 [details]
Bug 1873002 - Make the richlistbox contents not grow the richlistbox. r=Gijs

Approved for 122.0 RC1

Comment 22

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/daab7714d647

Comment 23

[Donal Meehan [:dmeehan]]

:emilio what do you think about uplifting to ESR115?

Comment 24

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9372501 [details]
Bug 1873002 - Make the richlistbox contents not grow the richlistbox. r=Gijs

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: comment 0
• User impact if declined: comment 0
• Fix Landed on Version: 122
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): See above.

Comment 25

[Donal Meehan [:dmeehan]]

Comment on attachment 9372501 [details]
Bug 1873002 - Make the richlistbox contents not grow the richlistbox. r=Gijs

Approved for 115.7esr.

Comment 26

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/29e552ccfa53

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dafb28a11ceb
Make dialog contents shrinkable even when not scrollable. r=Gijs,settings-reviewers,desktop-theme-reviewers,dao

Comment 28

[Sandor Molnar[:smolnar]]

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

Comment 29

[Iulian Moraru]

Backed out changeset dafb28a11ceb (bug 1873002) for causing Bug 1874824.(on request from Emilio)

• Backout link

Comment 30

[Hani Yacoub, Desktop QA]

Verified as fixed on Windows 10 x64, macOS 11.6 and on Ubuntu 20.04 x64.