Closed Bug 677539 Opened 13 years ago Closed 13 years ago

Bookmarking group of tabs greyed out after restart with session restore

Categories

(SeaMonkey :: Bookmarks & History, defect)

defect
Not set
normal

Tracking

(seamonkey2.3 wontfix, seamonkey2.4 fixed, seamonkey2.5 fixed)

RESOLVED FIXED
seamonkey2.5
Tracking Status
seamonkey2.3 --- wontfix
seamonkey2.4 --- fixed
seamonkey2.5 --- fixed

People

(Reporter: prof, Assigned: InvisibleSmiley)

References

Details

Attachments

(1 file, 1 obsolete file)

When starting Seamonkey with at least 2 windows and 2 tabs in each window by using the option to restore the previous session, the menu entry to bookmark the group of tabs is greyed out from the second window onwards. (e.g. having 5 Windows, each containing several tabs, there are at least 4 windows which have this option greyed out. When opening another window with new tabs, the option is available/enabled in that window. The "old" windows remain as they are. Quitting SM by Ctrl+Q and restarting with that new window in addition to the old makes makes it behaving like the old ones of course. Illustration of the issue in a localized Seamonkey 2.2 http://img718.imageshack.us/img718/1752/gruppetabslesezeichen.png Hartmut Figge confirmed issue for SM2.5 Nighty in d.c.s.m.browser <4E4094CD.90009@hfigge.myfqdn.de> steps to reproduce: - use option to restore session in browser preferences as starting-page - open more than one window with more than one tab in each window - quit SM (e.g. by pressing Ctrl+Q) and restart - try to bookmark group of tabs in any window except the first one expected result: - Bookmarking tab group is enabled as usual actual result: - Bookmarking tab group is disabled
Confirmed, -safe-mode. Mozilla/5.0 (Windows NT 5.1; rv:6.0) Gecko/20110806 Firefox/6.0 SeaMonkey/2.3
Without looking too closely, appears that this has been an issue since SeaMonkey 2.1. SeaMonkey 2.0.14 is not affected.
FWIW, the command is Browser:BookmarkAllTabs (navigator.xul and/or navigatorOverlay.xul). From navigatorOverlay.xul: <!-- The command is disabled for the hidden window. Otherwise its enabled state is handled by gBookmarkAllTabsHandler. --> <command id="Browser:BookmarkAllTabs" label="&addCurTabsAsCmd.label;" accesskey="&addCurTabsAsCmd.accesskey;" oncommand="gBookmarkAllTabsHandler.doCommand();" disabled="true"/> gBookmarkAllTabsHandler is defined in navigator.js, reacting to tabOpen and tabClose events (and setting the disabled attribute to false in case of a tabOpen event with current tab count == 2). I guess in the case of Session Restore, the event listeners are added too late so the events are not received/processed by gBookmarkAllTabsHandler and subsequently the disabled state of the menuitem is not changed. Misak, Neil, what can we do about this? [I guess FF doesn't have a case for this; they don't have that menuitem in the menu anymore, only in tab context menus.]
One option would be to stop using observer notifications to track windows.
Maybe we should simply attach a trigger to the Bookmarks menu/popup (don't know if that's what Neil meant in comment 5).
Attached patch patch (obsolete) (deleted) — Splinter Review
I'll give this a try... Note: The tabbrowser updating is already fine since the updatePopupMenu method takes care of disabling all tbattr=tabbrowser-multiple items if there is only one tab.
Assignee: nobody → jh
Status: NEW → ASSIGNED
Attachment #552794 - Flags: review?(neil)
Bah, forgot the Bookmarks button menu (on the Personal Toolbar). Note: Using any tab's context menu serves as a workaround until this bug gets fixed (of course that doesn't bring the shortcut back to life; one could bind a call to PlacesCommandHook.bookmarkCurrentPages() to a shortcut using KeyConfig, though.
Attachment #552794 - Attachment is obsolete: true
Attachment #552794 - Flags: review?(neil)
Attachment #552796 - Flags: review?(neil)
Comment on attachment 552796 [details] [diff] [review] patch v1a [Checkin: comments 10 and 13] I have this vague memory that KaiRo wanted the gBookmarkAllTabsHandler for compatibility with Firefox. And then they went and removed it...
Attachment #552796 - Flags: review?(neil) → review+
Attachment #552796 - Attachment description: patch v1a → patch v1a [Checkin: comment 10]
Attachment #552796 - Flags: approval-comm-aurora?
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.5
Comment on attachment 552796 [details] [diff] [review] patch v1a [Checkin: comments 10 and 13] Just in case we still accept low risk regression fixes for 2.3, I'll request comm-beta approval, too.
Attachment #552796 - Flags: approval-comm-beta?
Comment on attachment 552796 [details] [diff] [review] patch v1a [Checkin: comments 10 and 13] sorry too late for 2.3
Attachment #552796 - Flags: approval-comm-beta? → approval-comm-beta-
Attachment #552796 - Flags: approval-comm-aurora? → approval-comm-aurora+
Attachment #552796 - Attachment description: patch v1a [Checkin: comment 10] → patch v1a [Checkin: comments 10 and 13]
Depends on: 682531
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: