245984 - Bookmarks context menu (Personal Bar & Bookmarks Manager) lacks mnemonics (access-keys/underlined keyboard shortcuts)

Status: VERIFIED FIXED

(Firefox :: Menus, defect, P3)

Description

[Prognathous]

This is the Firefox equivalent of Bug 214881 (for Seamonkey).

Fixing this bug would not only make Firefox more accessible, but it would also
minimize the effect of bugs like Bug 172675 (by making it almost as quick to
open a folder in new tabs).

Prog.

Comment 1

[Prognathous]

Attachment: Bookmarks context menu mnemonics: Firefox 0.9rc vs. IE6

If parity with IE is still sought after, I suggest to follow the same mnemonics
(where possible):

  &Open
  Cu&t
  &Copy
  &Delete
  P&roperties <- not sure about this one, perhaps &Properties would be better.

The rest are not available in IE and should probably follow other Firefox
menus.

Prog.

Comment 2

[:deanis74]

P&roperties is what Windows Explorer uses, but IE uses &Properties.  We use
&Properties elsewhere, like in a link context menu so we should probably stay
with that.

Comment 3

[:deanis74]

Prog, what about doing up a patch that uses &Properties?  We'll at least be
consistent throughout the app.  Maybe with an actual patch, we can work with
Mike to drive it in for 1.0.

Comment 4

[Mike Connor [:mconnor]]

access fixes are always a good thing, I'll buy that for a dollar

and &Properties would be my preference.

Comment 5

[Prognathous]

Ok, so we're settled on &Properties. There's a slight problem though - it's
already defined as "i" and used by the the new Bookmarks Manager:

http://lxr.mozilla.org/aviarybranch/source/browser/locales/en-US/chrome/browser/bookmarks/bookmarks.dtd#47
http://lxr.mozilla.org/aviarybranch/source/browser/components/bookmarks/content/bookmarksManager.xul

Prog.

Comment 6

[:deanis74]

Probably done only because Edit > Paste uses 'P' already.  Can we have Edit >
Properties still use 'i' as an access key and the context menu and toolbar use
'P'?  Or would that be too confusing?

Comment 7

[José Jeria]

*** Bug 253106 has been marked as a duplicate of this bug. ***

Comment 8

[Asaf Romano (gone)]

Attachment: Enable use of mnemonics for bookmarks

As we are not dealing with a xul+dtd file (bookmarks.properties), we need a way
to define and get access keys.

After landing this patch, we can add lines like the following to
bookmarks.propeties:
accesskey_bm_properties = i

(that's for cmd_bm_properties)

Comment 9

[Asaf Romano (gone)]

Attachment: Patch with the mnemonics

Comment 10

[Asaf Romano (gone)]

*** Bug 176359 has been marked as a duplicate of this bug. ***

Comment 11

[Asaf Romano (gone)]

Prog, about what bug have you talked in comment 0? bug 214881 is not the one...

Comment 12

[Prognathous]

It was supposed to be Bug 176359, not 214881. My clipboard was probably holding
that value for the this-bug-blocks field... ;-)

It seems that 'P' and 'R' are already taken (for Paste and Rename), but 'i' for
Properties is just too narrow. How about 'm' for Rename, 'r' for Properties, and
'g' for Manage Folder? At least all three are inline with Windows Explorer
mnemonics (if anyone was wondering, 'g' is used in My Computer context menu for
Manage).

Prog.

Comment 13

[Asaf Romano (gone)]

Prog, we use "M" as the mnemonic for "Manage" in the Bookamrks menu (main window).

Comment 14

[Jeremy M. Dolan]

 cmd_bm_openfolder = Open in Tabs
-cmd_bm_openfolder_accesskey = O
+cmd_bm_openfolder_accesskey = p

How about 'T' for this one? Better to use the first letter of a major word for
mnemonics. Or maybe even continue to use "O" here. Not many people use the
regular open in a context menu (they just do regular click), so give that the
crummy 'p'. In fact, there's a bug filed to have 'Open' removed altogether.

Comment 15

[Prognathous]

Perhaps I wasn't clear enough. The idea is to use 'g' for Manage, so that 'm'
becomes free for Rename, so that 'r' becomes free for Properties. Get it?

Prog.

Comment 16

[Asaf Romano (gone)]

(In reply to comment #14)
I agree for "T".

...(As we use this for "Open in new Tab" (obviously, they are not available at
the same time...))


(In reply to comment #15)
I just said we already use "m" for "Manage" in other places.

Comment 17

[Asaf Romano (gone)]

:-) We can safely use "r" without cahnging anything... The rename command
doesn't appear on the relevant menus.

Comment 18

[Asaf Romano (gone)]

We can'tm use "T" for "Open in Tabs" and not for "Open in New Tab", that's
cu&t...sorry, my mistkae

Comment 19

[Asaf Romano (gone)]

Attachment: Some changes

"P&roperties": but not in Bookmarks Manager ("&Redo").
I restored "&Open in tabs" ("&Open" and "&Open in tabs" items don't appear in
the same time).

Comment 20

[Asaf Romano (gone)]

Comment on attachment 154593 [details] [diff] [review]
Some changes

..and I forgot to mention "Open in Ne&w Tab", this is not so good...but morst
of us ctrl+click the bookmark.
(that's because of cu&t).

Comment 21

[Ben Goodger (use ben at mozilla dot org for email)]

l10n impact -> +, please get mconnor's review and land within 7 days otherwise
this will be -. 

Comment 22

[Asaf Romano (gone)]

Attachment: Up-to-date patch

I hope i won't need to create it another time...

Comment 23

[Jeremy M. Dolan]

mconnor, can you please take a look at this fairly simple patch? Ben says it has
to be in by Sunday/Monday or it will be -'d due to l10n freeze.

Comment 24

[Mike Connor [:mconnor]]

Comment on attachment 155335 [details] [diff] [review]
Up-to-date patch

looks good to me, had to update the patch to current CVS (vlad bitrotted you). 
Will attach a current patch in a moment

Comment 25

[Mike Connor [:mconnor]]

Attachment: CVS tip version of patch for aviary, just needs approval-aviary.

Comment 26

[Asaf Romano (gone)]

Comment on attachment 155467 [details] [diff] [review]
CVS tip version of patch for aviary, just needs approval-aviary.

moving review from mconnor, asking approval

Comment 27

[Asaf Romano (gone)]

Asa, can you grant it? thanks!

Comment 28

[Asa Dotzler [:asa]]

Sairuh, or Aaron, can you take a quick look at this change? If there are no
obvious problems/conflicts, I'd like to see it land soon.

Comment 29

[sairuh (rarely reading bugmail)]

the mnemonics in the patch look fine to me, but AaronL would be the best person
for a thorough r=.

Comment 30

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 155467 [details] [diff] [review]
CVS tip version of patch for aviary, just needs approval-aviary.

a=[e-mail]

Comment 31

[Aaron Leventhal]

Looks good.

Comment 32

[Asaf Romano (gone)]

now, can someone check it in (Trunk & Aviary)? Thanks.

Comment 33

[Ben Goodger (use ben at mozilla dot org for email)]

Done. 

Comment 34

[Ryan Polk (Quark)]

setting fixed-aviary1.0 for bugfixes checked into branch, for searching purposes.
sorry for bugspam.

Comment 35

[sairuh (rarely reading bugmail)]

in bookmarks toolbar and bookmarks manager context menus, these two accesskeys
conflict (redundant S):

New _Separator
_Sort by Name

tested with 2004100609-0.9+ (linux fc2). has another bug been filed on that yet?

Comment 36

[Robin Monks]

*** Bug 276950 has been marked as a duplicate of this bug. ***