370456 - Missing kbd accelerator Bookmarks->Organize Bookmarks...

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Mats Palmgren (inactive)]

From bug 370453:

WARNING: Key 'historyHome' of menu item 'Home' could not be found: file
nsMenuFrame.cpp, line 1520
WARNING: Key 'manBookmarkKb' of menu item 'Organize Bookmarks...' could not be
found: file nsMenuFrame.cpp, line 1520

Comment 1

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

CTRL+B opens the bookmarks sidebar so I chose Shift+same for Organize Bookmarks.
Home is Alt+Home as it has always been, although there is some discussion
in bug 231248 that it should be something else on MacOSX...

Comment 2

[Asaf Romano (gone)]

Comment on attachment 255161 [details] [diff] [review]
Patch rev. 1

Neil isn't working on/reviewing browser/ code.

I would rather fix the places menu to use the old IDs.

Comment 3

[Mats Palmgren (inactive)]

Attachment: Patch rev. 2

Comment 4

[Asaf Romano (gone)]

Comment on attachment 255163 [details] [diff] [review]
Patch rev. 2

I didn't realize you're adding a keyboard shortcut here.

Please request ui-r from beltzner first or spin that off to a new bug and just remove the key attribute.

Comment 5

[Mats Palmgren (inactive)]

Comment on attachment 255163 [details] [diff] [review]
Patch rev. 2

This patch will bind "Bookmarks->Organize Bookmarks..." to Accel+Shift+B.
Accel+B is currently bound to "open bookmarks in sidebar" so I thought
adding Shift would be a good choice to open it in a separate window.

Comment 6

[Asaf Romano (gone)]

Isn't Accel+Shift+[A-E] reserved on gtk2 though?

Comment 7

[[e-mail]]

(In reply to comment #6)
>Isn't Accel+Shift+[A-E] reserved on gtk2 though?
Accel+Shift+[0-9A-F], yes...

Comment 8

[(not reading, please use [e-mail] instead)]

jminta has fixed the history->home part of this bug, see bug #381393 (a dup that I logged, sorry)

for the bookmarks->organize, I logged another dup, see bug #381392

Comment 10

[(not reading, please use [e-mail] instead)]

morphing bug.

history is fixed, and fx 2 didn't have a kbd accelerator for bookmarks, organized.

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 255163 [details] [diff] [review]
Patch rev. 2

ui-r+, with a request to please buy some more keys for the standard keyboard, we're running kinda low!

Comment 12

[(not reading, please use [e-mail] instead)]

Comment on attachment 255163 [details] [diff] [review]
Patch rev. 2

sending review to mano, now that we have ui-r=+

Comment 13

[Asaf Romano (gone)]

Comment on attachment 255163 [details] [diff] [review]
Patch rev. 2

MOZ_PLACES_BOOKMARKS is gone.

Comment 14

[(not reading, please use [e-mail] instead)]

dietrich is on it!

Comment 15

[Dietrich Ayala (:dietrich)]

Attachment: Patch rev. 3

Comment 16

[(not reading, please use [e-mail] instead)]

Comment on attachment 280483 [details] [diff] [review]
Patch rev. 3

r=sspitzer, and tested on windows xp.

specifically, ctrl+shift+b opens the bm organizer and it wasn't anything before (as far as I can tell, with an older build)

Comment 17

[Dietrich Ayala (:dietrich)]

Comment on attachment 280483 [details] [diff] [review]
Patch rev. 3

requesting approval: this is a small change that will ease (and likely increase) usage of the organizer.

Comment 18

[Marcia Knous [:marcia]]

Litmus Triage Team: Adding abillings and twalker to this bug to cover the test case portion.

Comment 19

[Asaf Romano (gone)]

Comment on attachment 280483 [details] [diff] [review]
Patch rev. 3

as i noted above, this isn't going to work on gtk, you should #ifdef it out at the very least...

Comment 20

[(not reading, please use [e-mail] instead)]

> this isn't going to work on gtk
> Isn't Accel+Shift+[A-E] reserved on gtk2 though?

oops, thanks for the reminder, mano.  

here comes a new patch.

Comment 21

[(not reading, please use [e-mail] instead)]

Attachment: Patch rev. 4

Comment 22

[Asaf Romano (gone)]

Comment on attachment 280705 [details] [diff] [review]
Patch rev. 4

r=mano

Comment 23

[Dietrich Ayala (:dietrich)]

Comment on attachment 280705 [details] [diff] [review]
Patch rev. 4

see comment #17

Comment 24

[(not reading, please use [e-mail] instead)]

fix landed on dietrich behalf.

Checking in browser/base/content/browser-sets.inc;
/cvsroot/mozilla/browser/base/content/browser-sets.inc,v  <--  browser-sets.inc
new revision: 1.103; previous revision: 1.102
done

Comment 25

[Aakash Desai [:aakashd]]

Test cases were created to regression test this bug on 3.x releases.

For 3.0 test runs,
https://litmus.mozilla.org/show_test.cgi?id=7494

For 3.1 test runs,
https://litmus.mozilla.org/show_test.cgi?id=7495

Comment 26

[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