Last Comment Bug 622010 - Tab context menu "Bookmark this group of tabs" doesn't work (Error: addGroupmarkAs is not defined)
: Tab context menu "Bookmark this group of tabs" doesn't work (Error: addGroupm...
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Bookmarks & History (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.1b3
Assigned To: Ian Neal
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-12-29 15:38 PST by Karsten Düsterloh
Modified: 2011-02-14 21:09 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Fix as suggested v0.1 [Checked in: Comment 5] (1.09 KB, patch)
2011-02-12 08:07 PST, Ian Neal
philip.chee: review+
Details | Diff | Splinter Review

Description Karsten Düsterloh 2010-12-29 15:38:31 PST
Open several tabs, rightclick on one of them, choose "Bookmark this group of tabs". Nothing happens but an error message on the Error Console:

Error: addGroupmarkAs is not defined
Source File: chrome://navigator/content/navigator.xul
Line: 1

Adding a bookmark group from the Bookmarks menu or Personal Toolbar's Bookmarks button works.
Comment 1 Philip Chee 2011-01-01 03:20:18 PST
> Error: addGroupmarkAs is not defined
Probably something like:
gBookmarkAllTabsHandler.doCommand();
Comment 2 Ian Neal 2011-02-12 08:07:04 PST
Created attachment 511959 [details] [diff] [review]
Fix as suggested v0.1 [Checked in: Comment 5]

Simple fix
Comment 3 Philip Chee 2011-02-13 19:07:12 PST
Comment on attachment 511959 [details] [diff] [review]
Fix as suggested v0.1 [Checked in: Comment 5]

Works. I thought that we might need to use command="Browser:BookmarkAllTabs" but it turns out that the tabs context menu has its own disabled items handler.
Comment 4 Philip Chee 2011-02-13 19:10:56 PST
> Works. I thought that we might need to use command="Browser:BookmarkAllTabs"
> but it turns out that the tabs context menu has its own disabled items handler.

In which case it would probably be simpler to call 
PlacesCommandHook.bookmarkCurrentPages() directly in the onbookmarkgroup=.

r+=me either way.
Comment 5 Ian Neal 2011-02-14 14:15:37 PST
Comment on attachment 511959 [details] [diff] [review]
Fix as suggested v0.1 [Checked in: Comment 5]

http://hg.mozilla.org/comm-central/rev/1514f799efdb

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