Last Comment Bug 589592 - [SeaMonkey 2.1, mochitest-browser-chrome] Double id='addBookmarkKb' detected in navigator.xul
: [SeaMonkey 2.1, mochitest-browser-chrome] Double id='addBookmarkKb' detected ...
Status: RESOLVED FIXED
[sm-perma][cc-orange]
: intermittent-failure
Product: SeaMonkey
Classification: Client Software
Component: Bookmarks & History (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.1b1
Assigned To: Robert Kaiser
:
:
Mentors:
Depends on: SMPlacesBMarks
Blocks: SmTestFail
  Show dependency treegraph
 
Reported: 2010-08-22 12:24 PDT by Robert Kaiser
Modified: 2012-11-26 02:57 PST (History)
2 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
remove platform-specific keys (3.55 KB, patch)
2010-08-22 13:01 PDT, Robert Kaiser
neil: review+
Details | Diff | Splinter Review

Description Robert Kaiser 2010-08-22 12:24:03 PDT
This probably comes from places bookmarks:

Double id='addBookmarkKb' detected in chrome://navigator/content/navigator.xul:
  Tree 0:
    key addBookmarkKb [39]
    keyset navKeys [24]
    window main-window [25]
    #document undefined [0]
  Tree 1:
    key addBookmarkKb [9]
    keyset navigationKeys [48]
    keyset navKeys [24]
    window main-window [25]
    #document undefined [0]
ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/chrome/suite/common/tests/chrome/test_idcheck.xul | check id: navigator.xul#addBookmarkKb


Thanks to Callek for finding that one!
Comment 1 Robert Kaiser 2010-08-22 12:54:07 PDT
http://mxr.mozilla.org/comm-central/search?string=addBookmarkKb tells a pretty good story. I guess we should kill it from platformNavigationBindings.xul or navigatorOverlay.xul - but as it's only on the former in non-Linux, I guess that's what we should change.
Comment 2 Robert Kaiser 2010-08-22 13:01:12 PDT
Created attachment 468166 [details] [diff] [review]
remove platform-specific keys

We have accel,shift in use for tabs already now, so this is a conflict as well. The best thing is to remove the platform-specific bindings, from what I'm seeing.
Comment 3 neil@parkwaycc.co.uk 2010-08-22 13:09:11 PDT
Comment on attachment 468166 [details] [diff] [review]
remove platform-specific keys

Fortunately the Ctrl+Shift+D shortcut is no longer reserved by GTK. (Sadly you're now trying to use the reserved Ctrl+Alt+D shortcut. Doesn't anyone read the Error Console any more? I really need to review more of those Places patches...)
Comment 4 Robert Kaiser 2010-08-22 16:23:43 PDT
Pushed as http://hg.mozilla.org/comm-central/rev/64ad57c8da27

(In reply to comment #3)
> (Sadly
> you're now trying to use the reserved Ctrl+Alt+D shortcut. Doesn't anyone read
> the Error Console any more? I really need to review more of those Places
> patches...)

I read it but didn't find a different combination to use for something I wasn't sure if it would even stick. And actually, it might be a good idea to just remove that option when the fast bookmarking button lands. Though I wonder if FF has a shortcut for that one...
Comment 5 neil@parkwaycc.co.uk 2010-08-23 16:30:56 PDT
(In reply to comment #4)
> And actually, it might be a good idea to just
> remove that option when the fast bookmarking button lands.
The fast bookmarking button doesn't help keyboard users, who are the people interested in the shortcut...
Comment 6 Robert Kaiser 2010-08-24 05:16:57 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > And actually, it might be a good idea to just
> > remove that option when the fast bookmarking button lands.
> The fast bookmarking button doesn't help keyboard users, who are the people
> interested in the shortcut...

Well, the question is if they need shortcuts for both "add bookmark without being prompted" and "add bookmark with an add bookmark panel showing".
Comment 7 Justin Wood (:Callek) 2010-08-24 09:39:12 PDT
I'd say keyboard shortcut for "Add Bookmark" should trigger fast path, second invoke of keyboard shortcut should prompt (also allowing remove).
Comment 8 Robert Kaiser 2010-08-24 09:45:12 PDT
(In reply to comment #7)
> I'd say keyboard shortcut for "Add Bookmark" should trigger fast path, second
> invoke of keyboard shortcut should prompt (also allowing remove).

This basically means putting the shortcut on the icon and removing the one on the "..." menu item. I also think the other menu item for simple adding should go away. Still, let's put this on a different bug.

Note You need to log in before you can comment on or make changes to this bug.