1241614 - No "View Certificate" button for SSL errors?

Status: RESOLVED FIXED

(Core Graveyard :: Security: UI, defect)

Description

[Justin Dolske [:Dolske]]

1) Load https://self-signed.badssl.com/
2) Get error page, click Advanced
3) Click "Add exception"

In the override dialog that comes up, there used to be a "View Certificate" button. This has been useful for user support in the past, because when they're being MITM'd by crappy antivirus, the AV name if often listed in the cert.

I don't see this button on Nightly 46, but it's still there on DevEdition 45. Did we intentionally disable this, or is it just broken?

Comment 1

[Justin Dolske [:Dolske]]

Huh. Curious, I don't see this in a new profile, but I do in my normal profile. Even after a restart?

Comment 2

[Justin Dolske [:Dolske]]

Ok, so after cloning my profile and chopping away pieces, I found that deleting xulstore.json makes the button reappear. Poking further, I can just neuter the following entry to fix it:

   "chrome://pippki/content/exceptionDialog.xul":{
      "exceptiondialog":{
         "width":"501",
         "screenX":"356",
         "screenY":"79",
         "height":"237"
      }
   },

So that's weird. Is it trying to persist a tiny window, and hiding the cert status section to fit in the available space? O_o

Comment 3

[Justin Dolske [:Dolske]]

Ugh, yep. The sheet (on OS X) is actually resizeable, and making it bigger reveals the missing content! It's tricky because when shrunken to the smaller size, there are not even any scrollbars or a scrollable region.

Wonder if we should slap a min-height in here? Or just remove the persist entirely?

Comment 4

[:Gijs (he/him)]

(In reply to Justin Dolske [:Dolske] from comment #3)
> Ugh, yep. The sheet (on OS X) is actually resizeable, and making it bigger
> reveals the missing content! It's tricky because when shrunken to the
> smaller size, there are not even any scrollbars or a scrollable region.

I mean, there are, but they only show on hover and the like. That should be fixable though, probably by overriding:

https://dxr.mozilla.org/mozilla-central/rev/c5da92c5b4906369dee83629f81d647226ac1038/toolkit/themes/osx/global/nativescrollbars.css#26-31

> Wonder if we should slap a min-height in here? Or just remove the persist
> entirely?

I think a min-height on that inner resizable box makes sense. I can see a downside to removing the persist attribute entirely, even if I personally wouldn't care.

Comment 5

[:Gijs (he/him)]

The overflow: auto on the vbox was added in bug 413623. On the whole I agree with the last comment (by Neil) that if we sized the box in em/ch or whatever, we wouldn't be having the original problem and we could dispense with the overflow:auto.

It does seem that in sheets on OS X, if you use height: rather than min-height in CSS, the buttons still get to fall off the bottom if you resize (seriously? Yes, seriously...). Anyway, I have a patch that I believe works OK, coming up...

Comment 6

[:Gijs (he/him)]

Attachment: MozReview Request: Bug 1241614 - don't overflow:auto the container, use em to size the dialog to avoid hidpi visibility issues, r?dolske,ttaubert

Review commit: https://reviewboard.mozilla.org/r/31931/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/31931/

Comment 7

[Justin Dolske [:Dolske]]

(In reply to :Gijs Kruitbosch from comment #4)
> (In reply to Justin Dolske [:Dolske] from comment #3)
> > Ugh, yep. The sheet (on OS X) is actually resizeable, and making it bigger
> > reveals the missing content! It's tricky because when shrunken to the
> > smaller size, there are not even any scrollbars or a scrollable region.
> 
> I mean, there are, but they only show on hover and the like.

Actually not in my case! However my sheet got resized, it was at the smallest height possible (ie, can't be sized smaller), and the scrollbars don't appear on hover. They do when it's larger. Only the "Permanently store this exception" checkbox/label is visible in the Certificate Status section.

Comment 8

[Justin Dolske [:Dolske]]

Comment on attachment 8710954 [details]
MozReview Request: Bug 1241614 - don't overflow:auto the container, use em to size the dialog to avoid hidpi visibility issues, r?dolske,ttaubert

https://reviewboard.mozilla.org/r/31931/#review29073

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8710954 [details]
MozReview Request: Bug 1241614 - don't overflow:auto the container, use em to size the dialog to avoid hidpi visibility issues, r?dolske,ttaubert

https://reviewboard.mozilla.org/r/31931/#review29155

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/c6bd409ec655

Comment 11

[Carsten Book [:Tomcat]]

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