1781922 - Icon is clipped on the bottom at SSL Network-Error pages

Status: VERIFIED FIXED

(Firefox :: Security, defect)

Description

[Daniel Holbert [:dholbert]]

STR:

1. Load https://wrong.host.badssl.com/
2. Look at the icon to the left of the text (yellow-triangle-sign with exclamation point).

ACTUAL RESULTS: The icon is clipped at the bottom.
EXPECTED RESULTS: No such clipping.

mozregression says this was regressed by bug 1737043 -- regression range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=00c4c63dc51ffbc60674c92e91d0212daa3de7da&tochange=c6cafb2781ca86d06b843adfea89934503f995e9

(And indeed, it looks like that commit changed various bits of neterror markup/styling.)

Comment 1

[Daniel Holbert [:dholbert]]

[Tracking Requested - why for this release]: unintended visual regression in our UI, which made it to beta but which we have the opportunity to fix (even via backout if needed, worst-case?) before release

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: screenshot showing the bug (from a mozregression-launched build shortly after the regression)

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: screenshot showing the old/correct rendering (from a mozregression-launched build shortly before the regression)

Comment 4

[Daniel Holbert [:dholbert]]

Looks like this behavior-change was specifically caused by the last line in this rule:

.title-text {
  font-weight: 300;
  line-height: 1.2;
  margin-bottom: 0;
  padding-bottom: 0;
}

https://searchfox.org/mozilla-central/rev/2be8794dd98b299fcfc5ce66e794e8b85ca14510/browser/themes/shared/aboutNetError.css#242

If I inspect the page in devtools and un-check that padding-bottom decl, then the we give the image a bit more space and don't clip it (i.e. the bug goes away) -- we use the padding-bottom: 0.4em value specified in info-pages.css instead:

.title-text {
  font-size: inherit;
  padding-bottom: 0.4em;
}

https://searchfox.org/mozilla-central/rev/2be8794dd98b299fcfc5ce66e794e8b85ca14510/toolkit/themes/shared/in-content/info-pages.css#62
(That's the value that we're using in old builds, before this regressed.)

We need this padding because the image is drawn using background-image, with background-size: 1.6em; making it draw the image as being nearly as tall as 2 lines of text. So we either need to reduce this background-size decl, or add back the padding-bottom, or otherwise ensure that we're giving the image enough space.

Comment 5

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

Set release status flags based on info from the regressing bug 1737043

Comment 6

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

The bug is marked as tracked for firefox104 (beta). However, the bug still isn't assigned.

:ckerschb, could you please find an assignee for this tracked bug? Given that it is a regression and we know the cause, we could also simply backout the regressor. If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit auto_nag documentation.

Comment 7

[:Gijs (he/him)]

James very courteously kept me as patch author but it wasn't really my patch anymore when it landed, so passing needinfo to James here.

Comment 8

[Christoph Kerschbaumer [:ckerschb]]

(In reply to :Gijs (he/him) from comment #7)

James very courteously kept me as patch author but it wasn't really my patch anymore when it landed, so passing needinfo to James here.

I guess no need for my ni? here ... since James most likely will tackle this one.

Comment 9

[James Teow [:jteow]]

Attachment: Bug 1781922 - Fix icon from being clipped on network-error pages - r?dao

Comment 10

[James Teow [:jteow]]

Thanks for flagging :dholbert

I was trying to mimic the Figma file that was provided to me but it might be better to keep the spacing consistent across the error pages especially seeing as how there was an unintended consequence such as this case and do more of a holistic tweak if spacing needs to change.

I've submitted a patch and will try uplifting it after it's approved since my change caused a visual regression but I feel should be a low-risk for uplifting.

Comment 11

[Dianna Smith [:diannaS]]

:jteow any update on landing this patch before early beta is over this week?

Comment 12

[James Teow [:jteow]]

:diannaS I was awaiting for the patch to be approved. It was approved so I'm in the midst of landing it and will request an uplift when its landed.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/62a4c33b5d15
Fix icon from being clipped on network-error pages - r=dao

Comment 14

[James Teow [:jteow]]

Comment on attachment 9287506 [details]
Bug 1781922 - Fix icon from being clipped on network-error pages - r?dao

Beta/Release Uplift Approval Request

• User impact if declined: Users will see a cut-off icon near the title text on certain error states in aboutNetError.xhtml
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Please see https://bugzilla.mozilla.org/show_bug.cgi?id=1781922#c0
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It's low risk because it's a CSS change and the removal of the overrides (that were added in the first place via the patch that caused the regression) will result in .title-text re-using the CSS it was using before the regression.
• String changes made/needed:
• Is Android affected?: No

Comment 15

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/62a4c33b5d15

Comment 16

[Bogdan Maris, Desktop Test Engineering [Back on 10th June]]

Reproduced the initial issue using old Nightly from 2022-07-27, verified that the icon is not cut anymore on the latest Nightly 105.0a1 across platforms (macOS 11.6, Windows 10 64bit and Ubuntu 22.04).

Comment 17

[Daniel Holbert [:dholbert]]

I also confirmed that this is fixed for me in latest Nightly, 105.0a1 (2022-08-03).

Thanks!

Comment 18

[Dianna Smith [:diannaS]]

Comment on attachment 9287506 [details]
Bug 1781922 - Fix icon from being clipped on network-error pages - r?dao

Approved for 106.0b6

Comment 19

[Dianna Smith [:diannaS]]

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

Comment 20

[Bogdan Maris, Desktop Test Engineering [Back on 10th June]]

Also verified fixed using latest Beta build from treeherder mozilla-beta branch on all platforms (Windows 10, macOS 11.6 and Ubuntu 18.04).