1799460 - Some of the toolbar context menu options are not completely displayed in high contrast mode

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Alin Ilea, Desktop Test Engineering]

Attachment: 2022-11-07_16h02_38.mp4

Found in

• 108.0a1 (2022-11-07)

Affected versions

• 108.0a1 (2022-11-07)

Tested platforms

• Affected platforms: Windows 7, Windows 10
• Unaffected platforms: macOS 12, Ubuntu 20

Steps to reproduce

1. Enable high contrast mode.
2. Launch Firefox and open the toolbar context menu.
3. Go to the Bookmarks Toolbar submenu.

Expected result

• Toolbar context menu items should be displayed correctly.

Actual result

• "Bookmarks Toolbar", "Only Show on New Tab" and "Never Show" options are not completely displayed.

Regression range

• First bad: aa325161aae5e555cab4d44b6662bbe029d26a5d
• Last good: 4ff88d45f59c4357dcc71ddf4ad5344a38f9944d
• Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=2815b08bfbd2f4abecf1e356461c0e2fc36bc025&tochange=fcb08a86da33aafb6527a040f598ce802d2d7a69
• Potentially regressed by: 1795199

Comment 1

[Dianna Smith [:diannaS]]

:emilio do you think you could take a look and see if this was caused by bug 1795199?

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Bug 1790616 most likely.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1799460 - Implement label[value] and start/end cropping with CSS rather than XUL layout. r=Gijs

This reduces the weird interactions that can appear on menus.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1799460 - Use width rather than min-width to set right/left menu sizes on windows menus. r=Gijs

This was hitting an interesting difference between XUL and non-XUL layout, when
you set min-width: <something> to something that also has non-default
appearance and imposes a min size.

This should behave consistently in both layout modes.

Depends on D161498

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1799460 - a11y fixes for my previous changes. r=Jamie

The button tweak is needed because now if you have:

<button>
<label value="foo">
</button>

There is a text node for the value (generated content), and the <button>
shouldn't steal it.

I was getting crashes without it because XULButtonAccessible isn't
hypertext (so I wonder if the IsText() code-path can even be reached?)

Setting crop="center" now reframes, so test_label needs to change.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1799460 - Patch for beta. r=Gijs

Let's use legacy layout in beta in native menupopups to fix this bug.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9303374 [details]
Bug 1799460 - Patch for beta. r=Gijs

Beta/Release Uplift Approval Request

• User impact if declined: comment 0 and duplicate
• 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 and duplicate bug
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): One-liner to revert to 107's codepath on beta for now.
• String changes made/needed: none
• Is Android affected?: No

Comment 9

[Dianna Smith [:diannaS]]

Comment on attachment 9303374 [details]
Bug 1799460 - Patch for beta. r=Gijs

Approved for 108.0b2

Comment 10

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/37fdf0b0db4c

Comment 11

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

The severity field for this bug is set to S4. However, the accessibility severity is higher, [access-s2].
:emilio, could you consider increasing the severity?

For more information, please visit auto_nag documentation.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Sure.

Comment 13

[Alin Ilea, Desktop Test Engineering]

Both bugs (this one and its duplicate) are verified - fixed in Beta 108.0b2 on Windows 10 and Windows 7. However, both issues are still reproducible in the latest Nightly 109.0a1 (2022-11-15). @Emilio do you know if these bugs will be also fixed in Nightly 109?
Thanks.

Comment 14

[Emilio Cobos Álvarez (:emilio)]

That's expected, proper fix hasn't landed on Nightly yet.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fa753c8bdf4c
Use width rather than min-width to set right/left menu sizes on windows menus. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/9da892f98d1b
Implement label[value] and start/end cropping with CSS rather than XUL layout. r=Gijs,jfkthame
https://hg.mozilla.org/integration/autoland/rev/fd69d6b3aeb8
a11y fixes for my previous changes. r=Jamie

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bcc0928c17b4
Annotate a test as passing in windows/android.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/392eeb01c8d5
Annotate a test as passing on Mac also.

Comment 18

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/fa753c8bdf4c
https://hg.mozilla.org/mozilla-central/rev/9da892f98d1b
https://hg.mozilla.org/mozilla-central/rev/fd69d6b3aeb8
https://hg.mozilla.org/mozilla-central/rev/bcc0928c17b4
https://hg.mozilla.org/mozilla-central/rev/392eeb01c8d5

Comment 19

[Alin Ilea, Desktop Test Engineering]

Re-tested using the latest Nightly build (build id: 20221118094451) on Windows 10 and the issue is still reproducible there. Emilio, could you please take a look over it?

Comment 20

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1799460 - Unset width in non-native menu-right. r=Gijs

Like we do for the left and so on.

Comment 21

[Emilio Cobos Álvarez (:emilio)]

Let's reopen for that.

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a8246e46adce
Unset width in non-native menu-right. r=desktop-theme-reviewers,dao

Comment 23

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/a8246e46adce

Comment 24

[Alin Ilea, Desktop Test Engineering]

Attachment: 2022-11-21_12h01_51.png

Both issues are still reproducible in the latest Nightly 109.0a1 (build id: 20221121093640), tested on Win10 (see the screenshot) and Win7 (the duplicate bug).

Comment 25

[Emilio Cobos Álvarez (:emilio)]

Bug 1801844 should take care of this for good.

Comment 27

[Catalin Sasca, Desktop Test Engineering [:csasca]]

Attachment: win 7 speed context menu.png

Worked on Win 7 and saw that there is still an issue on context menu when selecting the video speed as it is still not looking as it should. Here's an attachment with it on 109.0a1 (2022-12-08).

Comment 28

[:Gijs (he/him)]

(In reply to Catalin Sasca, QA [:csasca] from comment #27)

Created attachment 9307319 [details]
win 7 speed context menu.png

Worked on Win 7 and saw that there is still an issue on context menu when selecting the video speed as it is still not looking as it should. Here's an attachment with it on 109.0a1 (2022-12-08).

Is this tracked somewhere?

Comment 29

[Emilio Cobos Álvarez (:emilio)]

Pretty sure this was bug 1803773 effectively.

Comment 30

[Catalin Sasca, Desktop Test Engineering [:csasca]]

Logged Bug 1800228 initially on this, but was duped. I think the correct one now may be the one Emilio mentioned.