1738265 - Context menu and other popups open 4px to the right

Status: VERIFIED FIXED

(Core :: Widget: Gtk, defect)

Description

[Emilio Cobos Álvarez (:emilio)]

After bug 1738084, the context menu opens a bit too much to the right, because we don't offset the menupopup position by the box shadow.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1738265 - Add a negative margin to popups to account for the shadow added in bug 1738084. r=stransky

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1738265 - Improve some popup tests. r=stransky,NeilDeakin

This doesn't change behavior but I did it while debugging and I think it's
probably worth landing, as it shows the expected values rather than just the
fail message.

Depends on D129817

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1738265 - Teach some popup tests to deal with popups having margins by default. r=stransky,NeilDeakin

For tests that actually test margin handling I've just removed the default
margin by adding:

<?xml-stylesheet href="data:text/css,menupopup{margin: 0}" type="text/css"?>

The other tests I've just fixed by accounting for the margins.

Depends on D129865

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1738265 - Explicitly close some devtools menus in some tests. r=jdescottes,nchevobbe

With the negative margin, some context menus that remain open start
getting events that would otherwise go to the content underneath
(and which the affected tests need to pass).

Close the context menus explicitly in the affected tests (this is
what would usually happen if a user clicked a context menu item
anyways).

Depends on D129866

Comment 5

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

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

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/102ff2b893ea
Improve some popup tests. r=NeilDeakin
https://hg.mozilla.org/integration/autoland/rev/a2c91530a996
Teach some popup tests to deal with popups having margins by default. r=NeilDeakin

Comment 7

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/102ff2b893ea
https://hg.mozilla.org/mozilla-central/rev/a2c91530a996

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1738265 - Trivially fix other two popup tests that I had missed. r=NeilDeakin,stransky

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0cc74dde6707
Trivially fix other two popup tests that I had missed. r=stransky

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4c920e47260c
Fix two typos in the previous patch.

Comment 11

[Cristina Cozmuta (:CrissCozmuta)]

https://hg.mozilla.org/mozilla-central/rev/0cc74dde6707
https://hg.mozilla.org/mozilla-central/rev/4c920e47260c

Comment 12

[Emilio Cobos Álvarez (:emilio)]

[Tracking Requested - why for this release]: Popups are off on Linux without this fix. We could also consider backing out bug 1738084, but given we're early on the beta cycle uplifting this would be ideal.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8be1706fa428
Explicitly close some devtools menus in some tests. r=nchevobbe,devtools-reviewers

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e8a05519f047
Add a negative margin to popups to account for the shadow added in bug 1738084. r=stransky

Comment 15

[Iulian Moraru]

Backed out for causing mochitest failures on test_largemenu.html.

Push with failures

Failure log for c3
Failure log for c1

Backout link

This happened only on Linux 18.04 x64 WebRender debug.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/02a2d91ce5d2
Add a negative margin to popups to account for the shadow added in bug 1738084. r=stransky

Comment 17

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/8be1706fa428

Comment 18

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/02a2d91ce5d2

Comment 19

[Pascal Chevrel:pascalc]

Emilio, do you want to request uplift to beta? Thanks

Comment 20

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9248499 [details]
Bug 1738265 - Trivially fix other two popup tests that I had missed. r=NeilDeakin,stransky

Beta/Release Uplift Approval Request

• User impact if declined: Popups open too much to the right on Linux
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• 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): It's really just one-line CSS fix, but it needed a ton of test changes.
• String changes made/needed: none

Comment 21

[Pascal Chevrel:pascalc]

Comment on attachment 9248499 [details]
Bug 1738265 - Trivially fix other two popup tests that I had missed. r=NeilDeakin,stransky

Approved for 95 beta 3, thanks.

Comment 22

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-beta/rev/b34751f484ed
https://hg.mozilla.org/releases/mozilla-beta/rev/b564bd41a577

Comment 23

[Pascal Chevrel:pascalc]

The first patches had landed on 95 nightly before the merge, I only uplifted the remaining patches that landed in 96 after the merge.

Comment 24

[Hani Yacoub, Desktop QA]

Verified as fixed on Ubuntu 20.04 x64 on Firefox Nightly 96.0a1 and on Firefox 95.0b2.