Closed Bug 589592 Opened 9 years ago Closed 9 years ago

[SeaMonkey 2.1, mochitest-browser-chrome] Double id='addBookmarkKb' detected in navigator.xul

Categories

(SeaMonkey :: Bookmarks & History, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.1b1

People

(Reporter: kairo, Assigned: kairo)

References

(Blocks 1 open bug)

Details

(Keywords: intermittent-failure, Whiteboard: [sm-perma][cc-orange])

Attachments

(1 file)

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!
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.
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.
Assignee: nobody → kairo
Status: NEW → ASSIGNED
Attachment #468166 - Flags: review?(neil)
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...)
Attachment #468166 - Flags: review?(neil) → review+
Blocks: SmTestFail
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...
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1b1
Flags: in-testsuite-
(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...
(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".
I'd say keyboard shortcut for "Add Bookmark" should trigger fast path, second invoke of keyboard shortcut should prompt (also allowing remove).
(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.
Whiteboard: [sm-perma][orange] → [sm-perma]
Whiteboard: [sm-perma] → [sm-perma][cc-orange]
You need to log in before you can comment on or make changes to this bug.