Closed Bug 1707203 Opened 4 years ago Closed 4 years ago

Missing space between Finish your account and Extension requires new Permissions notifications

Categories

(Firefox :: Menus, defect, P2)

Desktop
Unspecified
defect

Tracking

()

VERIFIED FIXED
90 Branch
Tracking Status
firefox88 --- disabled
firefox89 --- verified
firefox90 --- verified

People

(Reporter: rdoghi, Assigned: mhowell)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [proton-icons] [proton-appmenu-notifications] [priority:2b] [proton-uplift])

Attachments

(3 files)

Attached image notFixed.png (deleted) —

[Affected platforms]:
Platforms: All

[Steps to reproduce]

  1. Launch the Firefox browser and install a few older addons like :
    Install version 1.35.2 of Bitwarden – Free Password manager - https://addons.mozilla.org/en-US/firefox/addon/bitwarden-password-manager/versions/?utm_content=search&utm_medium=referral&utm_source=addons.mozilla.org
    install version 7.0.0 of New Tab Override - https://addons.mozilla.org/en-US/firefox/addon/new-tab-override/versions/?utm_content=recommended_fallback&utm_medium=referral&utm_source=addons.mozilla.org

  2. Set the extensions.update.interval - 10

  3. Restart the Browser and wait for the Menu notifications to be displayed

  4. Create a new account in order for the Finish Account setup notification to be displayed.

[Expected result]
There should a 4px space between the Extensions require new Permissions and Finish your account setup notification banners

[Actual result]
The Finish you account setup/ Youve Been Disconnected notifications are stretching to the Extensions require new permissions notifications.

Whiteboard: [proton-appmenu-notifications]
Priority: -- → P2
Whiteboard: [proton-appmenu-notifications] → [proton-icons] [proton-appmenu-notifications] [priority:2b]
Assignee: nobody → mhowell
Status: NEW → ASSIGNED

Turns out that some changes to how the margin on the accounts menu item works
have allowed us to fix this bug and also simplify how the bottom margin on
menu notifications works at the same time; now, just having margin below every
banner all the time means that we get the correct spacing between multiple
banners and also between the last banner and the accounts item.

Attached image updatebackgroundbanner.png (deleted) —

There seems to be some space missing between the banners and the Sign in button as well.

The patch here should fix the issue in comment 2 as well.

Pushed by mhowell@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/71ba5122272a Always unconditionally leave margin below app menu notifications. r=desktop-theme-reviewers,harry
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch

This issue is Verified as fixed in our latest Nightly 90.0a1 (2021-05-06) on Mac, Windows and Ubuntu.

The patch landed in nightly and beta is affected.
:mhowell, 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.

Flags: needinfo?(mhowell)

Comment on attachment 9220485 [details]
Bug 1707203 - Always unconditionally leave margin below app menu notifications. r=#desktop-theme-reviewers

Beta/Release Uplift Approval Request

  • User impact if declined: MR1 regression causing a cosmetic defect in app menu notification banners, where there's no spacing between them and the accounts menu item.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • 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 just a CSS fix to the margin on the menu notifications.
  • String changes made/needed:
Flags: needinfo?(mhowell)
Attachment #9220485 - Flags: approval-mozilla-beta?

The uplift needs PM approval for MR1

Flags: needinfo?(smohanty)
Flags: needinfo?(rtestard)
Whiteboard: [proton-icons] [proton-appmenu-notifications] [priority:2b] → [proton-icons] [proton-appmenu-notifications] [priority:2b] [proton-uplift]

Approved for PM Uplift

Flags: needinfo?(smohanty)

Comment on attachment 9220485 [details]
Bug 1707203 - Always unconditionally leave margin below app menu notifications. r=#desktop-theme-reviewers

Approved for 89 beta 11, thanks.

Attachment #9220485 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

This issue is Verified as Fixed in our latest Beta 89.0b11 on Windows Mac and Ubuntu.

Status: RESOLVED → VERIFIED
Flags: needinfo?(rtestard)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: