331414 - Accelerate keys conflict in bookmark organizer window.

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect, P3)

Description

[Tim Miao]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.8) Gecko/20050607 Firefox/1.0.4
Build Identifier: Mozilla/5.0 (X11; U; SunOS sun4u; en-US; rv:1.9a1) Gecko/20060322 Firefox/1.6a1

Accelerate keys conflict.

Reproducible: Always

Steps to Reproduce:
1. Launch firefox and expand Bookmarks menu.
2. Select Organize Bookmarks menu to open the bookmark manage window.
3. Press Alt+F to expand File menu in this popup window.

Actual Results:  
New folder window will popup instead of expanding File menu. This leads to fail opening top menu File in this window.

Expected Results:  
There should not be two command with same accelerate key.

This bug can be reproduced on Mozilla/5.0 (X11; U; SunOS sun4u; en-US; rv:1.9a1) Gecko/20060322 Firefox/1.6a1. 
System info:
Sun Blade 100, SNV_35/Vermilion_07.

Comment 1

[Leon Sha]

Attachment: Patch

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 215984 [details] [diff] [review]
Patch

Since this accesskey is used in multiple places (see http://lxr.mozilla.org/seamonkey/search?string=placesCmd_new%3Afolder ), this will cause conflicting accesskeys in the bookmark context menu ("O" for "Open" and "New Folder"). It's hard to tell which accesskeys are still available to be used, and even more difficult to tell which can conflict with each other given that they are all used in multiple places, so I don't have any suggestions on how to fix this properly.

Comment 3

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #2)
> this will cause conflicting accesskeys in the bookmark context menu

Though I suppose having conflicting access keys in the context menu might be better than having them in the main window - so if all else fails, it might still be worth landing that patch.

Comment 4

[Jim Jeffery not reading bug-mail 1/2/11]

IMO Alt+F is universal in Windows for 'File' access, thus Places window should follow same, and move other accelerator keys that are in conflict. 

Fairly common practice to use Alt+F Alt+C to close the window. 

Comment 5

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #4)
> IMO Alt+F is universal in Windows for 'File' access, thus Places window should
> follow same, and move other accelerator keys that are in conflict.

No one is disagreeing with that statement, that's why this bug was filed.

Comment 6

[Leon Sha]

(In reply to comment #3)
> (In reply to comment #2)
> > this will cause conflicting accesskeys in the bookmark context menu
> 
> Though I suppose having conflicting access keys in the context menu might be
> better than having them in the main window - so if all else fails, it might
> still be worth landing that patch.
> 

In bookmarks manager of firefox 1.5, the access keys of newfolder is different in different place. It's alt-o in file menu and main window, alt-f in context menu. Why not port all these key settings here. Also it is better for user to have the same user experience.

Comment 7

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #6)
> Why not port all these key settings here. Also it is better for user to
> have the same user experience.

Sure, that might be a good idea, but there are many new items that also need accesskeys, so I don't know how feasible that is.

Comment 8

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

A better solution might be to move the accesskey attribute off the <command> and onto each instance.

Comment 9

[changdong.cui]

Attachment: change the accesskey to "o" ,just same as firefox 1.0.7

Comment 10

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

Comment on attachment 217406 [details] [diff] [review]
change the accesskey to "o" ,just same as firefox 1.0.7

I don't think it's a good idea to add another command for a different command instantiation in the UI... I think it'd be better to remove the accesskey attribute from the <command> element entirely and place it on each of the pieces of UI that refer to the command. 

e.g.:

>     <command id="placesCmd_new:folder" 
>              label="&cmd.new_folder.label;"
>              oncommand="PlacesController.newFolder()"/>

>   <menuitem id="placesContext_new:folder" command="placesCmd_new:folder"
>             accesskey="&foo;" selection="mutable"/>

Comment 11

[changdong.cui]

Attachment: patch(remove accesskey attribute from command)

Comment 12

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

Comment on attachment 217542 [details] [diff] [review]
patch(remove accesskey attribute from command)

r=[e-mail]

Comment 13

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

fixed-on-trunk, fixed-1.8-branch

Thanks!

Comment 14

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv