463923 - show expanded technical details in expert mode (SSL error pages)

Status: VERIFIED FIXED

(Core Graveyard :: Security: UI, enhancement)

Description

[Przemyslaw Bialik]

follow-up from Bug 431826

Comment 1

[Przemyslaw Bialik]

Attachment: Fix v1

Comment 2

[Jesse Ruderman]

I think you want review from johnath, since you're changing his code and UI.

Comment 3

[Przemyslaw Bialik]

I've thought about this but he is not listed as a peer :| ?
http://www.mozilla.org/owners.html
(also gavin e-mail looks to be not current)

Comment 4

[:Gavin Sharp [email: [e-mail]]]

It's current, it's just not the one I use for Bugzilla. Johnathan can (and probably should) review changes to that code. Looks fine to me, though.

Comment 5

[Johnathan Nightingale [:johnath]]

bug 431826 included a browser-chrome test which tests the behaviour of the expert pref to ensure that we don't break it at some point in the future.  You probably don't need to concoct a whole new test here, but can you add this check to the existing test?  If some future do-gooder changes the structure of this page and breaks the "technicalContent" ID, it would be nice to catch it.

Thanks for the patch though, the code change looks fine.

Comment 6

[Przemyslaw Bialik]

Attachment: Fix v2

Changes:
- removes second toggle from expert + technical link case...
- updates the test

Comment 7

[Johnathan Nightingale [:johnath]]

Comment on attachment 347322 [details] [diff] [review]
Fix v2

Looks good to me.  You may need an actual browser peer in order to get approval to land this, but thanks again for the work.

Comment 8

[Przemyslaw Bialik]

Comment on attachment 347322 [details] [diff] [review]
Fix v2

Asking for official approval.

Comment 9

[Johnathan Nightingale [:johnath]]

you've got review, but if you want this to be checkin-needed, it will need explicit approval for landing, now that we are in beta freeze.  You can request approval1.9.1b2 on the patch, and one of the beta drivers will evaluate it.

Sorry, I know this process can seem burdensome, but when we're locking down for a release, we are deliberately... more burdensome.  :)

Comment 10

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 347322 [details] [diff] [review]
Fix v2

We'll wait until after beta for this.

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 347322 [details] [diff] [review]
Fix v2

a191=beltzner

Comment 12

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/85507cfcdda8

Comment 13

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e2f37849c76d

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Backed out of 1.9.1 to fix test failures:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.1/1228955981.1228960442.2915.gz#err0

Comment 15

[:Gavin Sharp [email: [e-mail]]]

Were the tests failing reliably? Why isn't it failing on the trunk? If it's just an unreliable test we should disable the test, not back out the entire patch.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The tests were failing constantly, although only on one Mac test box, and backing out the patch fixed the failure.

You may be right that it's not the patch's fault, but the sheriff doesn't have to be the one to figure that out.

Comment 17

[Dão Gottwald [:dao]]

Note that bug 416661 seemingly had the same issue.

Comment 18

[Przemyslaw Bialik]

Should I attach a patch without the test or there would be some investigation why mac test box is failing the test ?

Comment 19

[:Gavin Sharp [email: [e-mail]]]

It sounds like that tinderbox is just broken somehow. Someone should try relanding the patch, and if it acts up again they can just disable the test.

Comment 20

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/7fca0bc1d2ee

Comment 21

[Marcia Knous [:marcia]]

verified fixed on the 1.9.1 branch using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20081223 Shiretoko/3.1b3pre and  Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20081223 Shiretoko/3.1b3pre.

verified fixed on the trunk using  Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20081223 Minefield/3.2a1pre and  Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20081223 Minefield/3.2a1pre

We should update the existing Litmus test case so nominating for that as well.

Comment 22

[u279076]

Attachment: Screenshot of error page

Comment 23

[u279076]

Test case added for the overall look and feel of the error page.
https://litmus.mozilla.org/show_test.cgi?id=7742