1634247 - Separators in the Developer Tools menu are displayed as menuseparators

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect, P1)

Description

[Itiel]

Attachment: Screenshot

I missed this in bug 1532172.

Comment 1

[Itiel]

The code for the separators that come before the last one is here:
https://searchfox.org/mozilla-central/rev/7fd1c1c34923ece7ad8c822bee062dd0491d64dc/devtools/client/framework/browser-menus.js#236
and the code for the last one that comes before the "Work Offline":
https://searchfox.org/mozilla-central/rev/7fd1c1c34923ece7ad8c822bee062dd0491d64dc/devtools/startup/DevToolsStartup.jsm#572

Simply replacing "menuseparator" with "toolbarseparator" doesn't work in either of them (nothing is drawn and no <toolbarseparator> appear in the markup).
No errors in the console either.

Dão, what am I missing here?

Comment 2

[:Gijs (he/him)]

(In reply to Itiel from comment #1)

The code for the separators that come before the last one is here:
https://searchfox.org/mozilla-central/rev/7fd1c1c34923ece7ad8c822bee062dd0491d64dc/devtools/client/framework/browser-menus.js#236
and the code for the last one that comes before the "Work Offline":
https://searchfox.org/mozilla-central/rev/7fd1c1c34923ece7ad8c822bee062dd0491d64dc/devtools/startup/DevToolsStartup.jsm#572

Simply replacing "menuseparator" with "toolbarseparator" doesn't work in either of them (nothing is drawn and no <toolbarseparator> appear in the markup).
No errors in the console either.

Dão, what am I missing here?

The screenshot is of the hamburger menu, maybe you want the code at https://searchfox.org/mozilla-central/rev/54f965e51e4f77866bec42737978d40d4c1fdfc5/browser/components/customizableui/CustomizableUI.jsm#4374 ?

Comment 3

[Itiel]

Attachment: Bug 1634247 - Fix separators in the Web Developer menu r?dao

Comment 4

[Itiel]

(In reply to :Gijs (he/him) from comment #2)

The screenshot is of the hamburger menu, maybe you want the code at https://searchfox.org/mozilla-central/rev/54f965e51e4f77866bec42737978d40d4c1fdfc5/browser/components/customizableui/CustomizableUI.jsm#4374 ?

It all makes sense now, thanks!

Comment 5

[Rachel Tublitz [:rachel]]

Hi Itiel - it looks like you're actively working on this one; would you mind adding a priority + severity?

Comment 6

[Itiel]

(In reply to Rachel Tublitz [:rachel] from comment #5)

Hi Itiel - it looks like you're actively working on this one; would you mind adding a priority + severity?

I don't think I'm qualified for setting these.
Transferring the needinfo to Gijs on that.

Comment 7

[:Gijs (he/him)]

We're tracking this as a regression and it's assigned, and the result looks a bit worse on macOS (the separators disappear entirely on the default/light theme) so S2 / P1 feels right. Thanks!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/589cd0f24209
Fix separators in the Web Developer menu r=dao

Comment 9

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/589cd0f24209

Comment 10

[Pascal Chevrel:pascalc (PTO until April 26)]

Gijs, Itiel, given that this is a P1 77 regression and that he patch doesn't look scary, should we uplift it to beta? Thanks

Comment 11

[:Gijs (he/him)]

Comment on attachment 9145879 [details]
Bug 1634247 - Fix separators in the Web Developer menu r?dao

Beta/Release Uplift Approval Request

• User impact if declined: Broken-looking separators in some hamburger menus
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Open the help or web developer submenus in the hamburger menu and check the separators look good.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Small JS-only patch switching to the correct type of separator in the hamburger menu, we have ample beta runway.
• String changes made/needed: No

Comment 12

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9145879 [details]
Bug 1634247 - Fix separators in the Web Developer menu r?dao

Small patch with tests for a 77 UX regression, let's take it into beta; thanks!

Comment 13

[Hani Yacoub, Desktop QA]

Attachment: Screenshot 2020-05-12 at 17.06.05.png

I couldn't reproduce the issue on a Firefox Nightly build from 29-04-2020 on MacOS 10.15 and on Windows 10 x64. (Please see screenshot attached)

Are there any extra steps that need to be done to reproduce the issue?

Comment 14

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/9be73e5d236b

Comment 15

[Itiel]

(In reply to Hani Yacoub from comment #13)

Created attachment 9147622 [details]
Screenshot 2020-05-12 at 17.06.05.png

I couldn't reproduce the issue on a Firefox Nightly build from 29-04-2020 on MacOS 10.15 and on Windows 10 x64. (Please see screenshot attached)

Are there any extra steps that need to be done to reproduce the issue?

Hmm, can you try with a Nightly build from 2020-05-01?

Comment 16

[Hani Yacoub, Desktop QA]

Thank you! I managed to reproduce the issue.
Verified as fixed on MacOS 10.15, Windows 10 x64 and Ubuntu 18.04 x64 on the latest Firefox Nightly and on Firefox 77.0b5.