1731618 - "Proceed with caution" message icon in about:config overlaps text for small windows (also for https-only error page)

Status: VERIFIED FIXED

(Toolkit :: Themes, defect, P1)

Description

[Kestrel]

Attachment: Warning icon overlaps text

STR:

1. Visit about:config with browser.aboutConfig.showWarning = true.
2. Reduce the window width to less than 970px.

Yellow warning icon overlaps the main text.

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=3a17d14515b67aba453734857145a0099c292ce0&tochange=2d387f5532024412d849dd0306311e95a96aa3c6

Regressed by Bug 1699593.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: screenshot with error page for http-only-mode

This also affects the "HTTPS Only Mode" intermediate page.

STR to hit this issue for that page

1. Set about:config pref dom.security.https_only_mode to true
2. Visit http://http.badssl.com/
3. Shrink your browser so the text starts wrapping.

ACTUAL RESULTS: The icon overlaps the text.
EXPECTED RESULTS: The icon should probably jump up above the text, to match the behavior that we have on e.g. the 'expired SSL cert' error page and others (e.g. for https://expired.badssl.com/ )

Comment 2

[Daniel Holbert [:dholbert]]

[Tracking Requested - why for this release]: Broken Firefox frontend UI, resulting from some cleanup that didn't intend to change behavior (at first glance) in bug 1699593. Let's not ship this brokenness, if we can avoid it.

Comment 3

[Daniel Holbert [:dholbert]]

Noah or Micah, perhaps one of you could look into this?

Comment 4

[Daniel Holbert [:dholbert]]

Looking at the certificate error page as a reference, it's got the following markup which saves it from hitting this same problem, inside its @media (max-width: 970px) { media-query rule:

  body.certerror .title {
    /* !important is necessary here until Bug 1723718 is resolved */
    background-image: url("chrome://global/skin/icons/warning.svg") !important;
    background-position: top left;
    padding-top: 60px;
    margin-top: -60px;
  }

We need to add a similar rule for these other affected pages. If I add a simplified[1] version of this rule to one of the affected pages, manually using devtools style editor (inside the existing @media (max-width: 970px) section), then it fixes the issue.

[1] e.g. for about:config, I can open up devtools style editor and add the following inside of the info-pages.css rule for @media (max-width: 970px) {...}, and it fixes it the issue:

  .title {
    padding-top: 60px;
    margin-top: -60px;
  }

Comment 5

[[:mtigley] (she/her)]

(In reply to Daniel Holbert [:dholbert] from comment #2)

[Tracking Requested - why for this release]: Broken Firefox frontend UI, resulting from some cleanup that didn't intend to change behavior (at first glance) in bug 1699593. Let's not ship this brokenness, if we can avoid it.

We can probably do what's done for the warning icon on the "expired SSL cert" error page here: https://searchfox.org/mozilla-central/rev/15de05f0e6d841cbc2ac66f8dcad72ebdada47f6/browser/themes/shared/aboutNetError.css#262-281 and have them ported here so they can be reused by the other pages.

Do you think you can take a look at this, Noah? If not, I can pick this up :)

Comment 6

[Noah Pesta]

(In reply to Micah [:mtigley] (she/her) from comment #5)

(In reply to Daniel Holbert [:dholbert] from comment #2)

[Tracking Requested - why for this release]: Broken Firefox frontend UI, resulting from some cleanup that didn't intend to change behavior (at first glance) in bug 1699593. Let's not ship this brokenness, if we can avoid it.

We can probably do what's done for the warning icon on the "expired SSL cert" error page here: https://searchfox.org/mozilla-central/rev/15de05f0e6d841cbc2ac66f8dcad72ebdada47f6/browser/themes/shared/aboutNetError.css#262-281 and have them ported here so they can be reused by the other pages.

Do you think you can take a look at this, Noah? If not, I can pick this up :)

Yes, I will take this.

Comment 7

[Noah Pesta]

Attachment: Bug 1731618 - Fixed overlapping icons on some error pages. r=mhowell,mtigley

Comment 8

[Release mgmt bot [:marco/ :calixte]]

The severity field is not set for this bug.
:dao, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/86a7c7ce13bc
Fixed overlapping icons on some error pages. r=mtigley,desktop-theme-reviewers,dao

Comment 10

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/86a7c7ce13bc

Comment 11

[Release mgmt bot [:marco/ :calixte]]

The patch landed in nightly and beta is affected.
:pestanoa, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 12

[Noah Pesta]

(In reply to Release mgmt bot [:sylvestre / :calixte / :marco for bugbug] from comment #11)

The patch landed in nightly and beta is affected.
:pestanoa, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

This will not need an uplift

Comment 13

[Ryan VanderMeulen [:RyanVM]]

That seems in conflict with comment 2.

Comment 14

[Daniel Holbert [:dholbert]]

I confirmed that the latest Firefox Beta (94.0b4) is indeed affected by this bug, so I think this does indeed need an uplift (otherwise this bug will be a user-facing defect as part of the Firefox 94 release).

Comment 15

[Noah Pesta]

Comment on attachment 9244258 [details]
Bug 1731618 - Fixed overlapping icons on some error pages. r=mhowell,mtigley

Beta/Release Uplift Approval Request

• User impact if declined: "Proceed with caution" message icon will continue to overlap the text when the window is shrunk.
• Is this code covered by automated tests?: Unknown
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: 1. Set about:config pref dom.security.https_only_mode to true

2. Visit http://http.badssl.com/
3. Shrink your browser so the text starts wrapping.

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is not risky because it only affect a small amount of the UI. The changes are also only 2 lines of CSS that only effect a small number of pages.
• String changes made/needed:

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9244258 [details]
Bug 1731618 - Fixed overlapping icons on some error pages. r=mhowell,mtigley

Approved for 94.0b5.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

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

Comment 18

[Giorgia Nichita, Release Desktop QA]

Reproduced this issue on Win 10x64 using an affected Nightly build 94.0a1 (2021-09-15), with STR from comment 0, as well as from comment 1.

The issue is verified as fixed on latest Beta 94.0b5 and latest Nightly 95.0a1 under macOS 10.15, Windows 10x64 and Ubuntu 20.04.