1749390 - [Wayland] Bookmarks menu popup has margin at the top

Status: VERIFIED FIXED

(Core :: Widget: Gtk, defect)

Description

[Viktor Jägersküpper]

Attachment: bookmarks menu popup with margin at the top

When the bookmarks menu icon is placed on the right side of the toolbar, its popup has a certain margin at the top so that it is not attached to the button. This is a regression which I only noticed today unfortunately.

Result given by mozregression:
Last good revision: 15d728e6c31d95844e23d9a9569e6dab8a7afeae
First bad revision: 208b8d7b09f96b5e46cf3be1d57426b51bf59a6b
Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=15d728e6c31d95844e23d9a9569e6dab8a7afeae&tochange=208b8d7b09f96b5e46cf3be1d57426b51bf59a6b

This points to bug 1739383.

OS: Arch Linux
Seen with Gnome and KDE

Comment 1

[Martin Stránský [:stransky] (ni? me)]

Can you test latest nightly?
https://fedoraproject.org/wiki/How_to_debug_Firefox_problems?rd=Bug_info_Firefox#Testing_Mozilla_binaries
Also do you run X11 or Wayland?
Thanks.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

I can repro. Can take a look. If I make this rule use margin-bottom it works, so I probably messed up my signs in the regressing bug.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1749390 - Deal with mismatched margin-top/bottom correctly on Wayland popup code. r=stransky

And apply the same fix for left/right.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1749390 - Add debug support for mozilla::Side. r=#gfx-reviewers

This was useful so may as well land it. Need to use mozilla::Side in the
header because otherwise it is ambiguous with mozilla::ipc::Side, yay
C++.

Depends on D135562

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aa25264bddfe
Deal with mismatched margin-top/bottom correctly on Wayland popup code. r=stransky
https://hg.mozilla.org/integration/autoland/rev/3b3df9fb7bcf
Add debug support for mozilla::Side. r=gfx-reviewers,lsalzman

Comment 6

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/aa25264bddfe
https://hg.mozilla.org/mozilla-central/rev/3b3df9fb7bcf

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Beta approval when you get a chance.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9258412 [details]
Bug 1749390 - Deal with mismatched margin-top/bottom correctly on Wayland popup code. r=stransky

Beta/Release Uplift Approval Request

• User impact if declined: comment 0
• 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: comment 0
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Simple tweak to fix popups that have mismatched margins (which are ~none, except this one apparently :))
• String changes made/needed: none

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9258412 [details]
Bug 1749390 - Deal with mismatched margin-top/bottom correctly on Wayland popup code. r=stransky

Approved for 97.0b3.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/bb35b276f7f3
https://hg.mozilla.org/releases/mozilla-beta/rev/fe3c5c3346fc

Comment 11

[Bogdan Maris, Desktop Test Engineering]

I only have Ubuntu (on Wayland) at my disposal but I could not reproduce this issue using an affected build so I can't actually verify that this is indeed fixed on latest builds. Victor, would you mind verifying if this issue is fixed for you on the latest Nightly build and Beta build?

Comment 12

[Viktor Jägersküpper]

Verified fixed on 97.0b4 and on latest Nightly 98.0a1 build (20220117035808), tested with both Gnome and KDE on Arch Linux.

Comment 13

[Bogdan Maris, Desktop Test Engineering]

Thank you very much for your verification Viktor.