Support accesskeys on HTML5 context menu items

NEW
Unassigned

Status

()

enhancement
P3
normal
8 years ago
10 months ago

People

(Reporter: squib, Unassigned)

Tracking

(Blocks 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
In working on bug 680192 for Thunderbird, I noticed that HTML5 context menus don't support the accesskey attribute on their menuitems. Attached is a patch to do this; I didn't write any tests, since I'm not sure what kind of tests are expected for this, but I'm perfectly happy to write some tests if given some guidance.

One possible complication with this is that there's nothing stopping webpages from using the same accesskeys as built-in items. I'm not sure how much of an issue that is though.

(CCing :volkmar on bz's recommendation)
Assignee: nobody → squibblyflabbetydoo
Version: unspecified → Trunk
I think Jan will be more appropriate than me to comment on this bug: he wrote the HTML5 context menu patches.
For what I can judge, your patch seems ok. I think you can test it with a mochitest-chrome test. You should find some doc about that in MDN if you never wrote one.
(Reporter)

Comment 2

7 years ago
Comment on attachment 576095 [details] [diff] [review]
Support accesskeys

I'm finally coming back to this (I was busy with Thunderbird stuff). Jan, does this seem like the right thing to do? As I mentioned in comment 0, "One possible complication with this is that there's nothing stopping webpages from using the same accesskeys as built-in items. I'm not sure how much of an issue that is though."

For tests, I'm guessing we could add some checks for the accesskey around here? http://mxr.mozilla.org/mozilla-central/source/browser/base/content/test/test_contextmenu.html?force=1#619 . Is there a better place to test this?
Attachment #576095 - Flags: feedback?(Jan.Varga)

Updated

6 years ago
Blocks: 617528
(Reporter)

Comment 3

6 years ago
I'm unlikely to work on this anytime soon. Others are free to work on it, though!
Assignee: squibblyflabbetydoo → nobody

Comment 4

6 years ago
Comment on attachment 576095 [details] [diff] [review]
Support accesskeys

Review of attachment 576095 [details] [diff] [review]:
-----------------------------------------------------------------

This should work.

::: content/xul/content/src/nsXULContextMenuBuilder.cpp
@@ +142,5 @@
>    menuitem->SetAttr(kNameSpaceID_None, nsGkAtoms::label, label, false);
>  
> +  nsAutoString accesskey;
> +  aElement->GetAccessKey(accesskey);
> +  menuitem->SetAttr(kNameSpaceID_None, nsGkAtoms::accesskey, accesskey, false);

You should check for non-emptiness here, I think
Attachment #576095 - Flags: feedback?(Jan.Varga) → feedback+

Updated

10 months ago
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.