1285225 - No shortcut key available for Attributes option via the Inspector context menu

Status: VERIFIED FIXED

(DevTools :: Inspector, defect, P1)

Description

[Ada [:adalucinet]]

[Affected versions]:
- latest Nightly 50.0a1 (e10s on/off)

[Affected platforms]:
- Windows 10 64-bit
- Ubuntu 16.04 64-bit

[Steps to reproduce]:
1. Launch Firefox
2. Open the Inspector.
3. Right click on any node from the Markup View

[Expected result]: Keyboard shortcuts are displayed for every option available in the context menu.

[Actual result]: No shortcut available for Attributes option.

[Regression range]: 
- Last good revision: 52679ce4756c53fd88054a55da482291c26ef8db
- First bad revision: 9694e371363590c8dace6629dc4d57f1af7206f2
- Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=52679ce4756c53fd88054a55da482291c26ef8db&tochange=9694e371363590c8dace6629dc4d57f1af7206f2
- I guess this regression was triggered by bug 1266478; Brian, what's your input on this matter? Thanks in advance!

[Additional notes]:
- Screenshot → https://i.imgur.com/9IAtLLI.png
- On Mac OS X 10.11.1, none of the shortcuts are displayed, but all of them are working as expected when selected via keyboard; this behavior is present on both latest Aurora 49.0a2 and firefox 48 beta. Is this expected at all?

Comment 1

[Ada [:adalucinet]]

Attachment: Screenshot.png

Attached is the correct screenshot. Sorry for the noise!

Comment 2

[Brian Grinstead [:bgrins]]

Attachment: Bug 1285225 - Set accesskey and disabled properties on menuitems with submenus in Menu API;

Review commit: https://reviewboard.mozilla.org/r/62924/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/62924/

Comment 3

[Brian Grinstead [:bgrins]]

Have a patch up for review that should have a fix.  Here's a try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bc294cd41a30

Comment 4

[Julian Descottes [:jdescottes]]

Comment on attachment 8768903 [details]
Bug 1285225 - Set accesskey and disabled properties on menuitems with submenus in Menu API;

https://reviewboard.mozilla.org/r/62924/#review59988

Looks good, thanks!

::: devtools/client/framework/menu.js:112
(Diff revision 1)
> +      if (item.disabled) {
> +        menu.setAttribute("disabled", "true");
> +      }
> +      if (item.accesskey) {
> +        menu.setAttribute("accesskey", item.accesskey);
> +      }

While we're at it, should we also set the id on a submenu item if an id is provided ?

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/720cb43ca662
Set accesskey and disabled properties on menuitems with submenus in Menu API;r=jdescottes

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/720cb43ca662

Comment 7

[Ada [:adalucinet]]

Verified fixed using latest Nightly 50.0a1, under Windows 10 64-bit and Ubuntu 16.04 64-bit.
Although, the issue mentioned in comment 1/[Additional notes] number 2 is still applicable: under Mac OS X 10.11.1, there are no shortcuts visible, but all of them work as expected when selected via keyboard. 
Brian, is this expected at all? Is there any intended implementation on this matter under Mac?

Comment 8

[Brian Grinstead [:bgrins]]

(In reply to Alexandra Lucinet, QA Mentor [:adalucinet] from comment #7)
> Verified fixed using latest Nightly 50.0a1, under Windows 10 64-bit and
> Ubuntu 16.04 64-bit.
> Although, the issue mentioned in comment 1/[Additional notes] number 2 is
> still applicable: under Mac OS X 10.11.1, there are no shortcuts visible,
> but all of them work as expected when selected via keyboard. 
> Brian, is this expected at all? Is there any intended implementation on this
> matter under Mac?

Yeah, this is expected on OSX - the accesskeys should still work but they aren't visible in the UI

Comment 9

[Ada [:adalucinet]]

(In reply to Brian Grinstead [:bgrins] from comment #8)
> (In reply to Alexandra Lucinet, QA Mentor [:adalucinet] from comment #7)
> > Verified fixed using latest Nightly 50.0a1, under Windows 10 64-bit and
> > Ubuntu 16.04 64-bit.
> > Although, the issue mentioned in comment 1/[Additional notes] number 2 is
> > still applicable: under Mac OS X 10.11.1, there are no shortcuts visible,
> > but all of them work as expected when selected via keyboard. 
> > Brian, is this expected at all? Is there any intended implementation on this
> > matter under Mac?
> 
> Yeah, this is expected on OSX - the accesskeys should still work but they
> aren't visible in the UI

Thanks for the prompt reply! Marking here accordingly!