1721651 - Display warning triangle as the favicon of certificate error pages

Status: VERIFIED FIXED

(Firefox :: Security, enhancement)

Description

[:Gijs (he/him)]

Attachment: Spec

See attachment. We want to more clearly signal that these certificate error pages are warnings.

Comment 1

[:Gijs (he/him)]

Johann, at one point we had separate markup for network and cert error pages. It now appears to me that despite its name, https://searchfox.org/mozilla-central/source/browser/base/content/certerror/aboutNetError.xhtml is used for both cert errors and network errors. Is that right? Is there a plan to stop doing that, or at least rename the confusing paths?

Comment 2

[:Gijs (he/him)]

Attachment: themes-vs-yellow-warning.png

On a completely different note... Katie, can you provide some clarity about the colours you envisaged here? The favicon in the spec has subtly different colours in dark and light mode. This isn't easily possible because the error page can only define 1 favicon. I can set it at runtime from within the code running inside the error page, but that code has no idea whether the user has a light or dark theme applied.

Finally, the icon is almost completely invisible in the alpenglow theme (see screenshot comparing light/dark/alpenglow backgrounds with the same icon). I expect there will be other custom themes where the favicon will be completely lost, colour-wise. Is that an acceptable trade-off, and/or how would you like to proceed here?

Comment 3

[katieC]

Attachment: Screenshot_tabsWarningIcon_LightDarkThemes.png

Thanks for the visuals and explanation. Updated UX Figma specs with the non-colour dark and light versions of the warning triangle in the tabs. (colour matches tab text values)

Comment 4

[:Gijs (he/him)]

Attachment: Bug 1721651 - show a warning triangle favicon on certificate error pages, r?pbz

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c7a8038bc730
show a warning triangle favicon on certificate error pages, r=pbz

Comment 6

[Atila Butkovits]

Backed out for causing Xpcshell failures at test_bookmarks_html.js.

Backout link: https://hg.mozilla.org/integration/autoland/rev/0c6ed183354aad2c65c23a4366e31227e8e6d9ac

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&searchStr=xpc&revision=c7a8038bc7303dc3be88130bcc808c04f9923cab&selectedTaskRun=eEhxxIfqQqSJac6bdlGifw.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=346726374&repo=autoland&lineNumber=2999

Comment 7

[:Gijs (he/him)]

https://treeherder.mozilla.org/jobs?repo=try&revision=0675eeffe2ac86f0731f95d2e2971b3762b65f87

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4065d7740cec
show a warning triangle favicon on certificate error pages, r=pbz

Comment 9

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/4065d7740cec

Comment 10

[Petruta Horea [:phorea], Desktop QA]

Verified fixed using latest Nightly 92.0a1 under Win 10 64-bit, Ubuntu 18.04 64-bit and Mac OSX 10.14.

Comment 11

[Johann Hofmann [:johannh]]

To answer the question, we merged them a long time ago because they mostly shared the same code. Hard to say if that was the right decision :)