The default bug view has changed. See this FAQ.

Bookmarking group of tabs greyed out after restart with session restore

RESOLVED FIXED in seamonkey2.5

Status

SeaMonkey
Bookmarks & History
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Martin F., Assigned: InvisibleSmiley)

Tracking

Trunk
seamonkey2.5

SeaMonkey Tracking Flags

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

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
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

Comment 1

6 years ago
Confirmed, -safe-mode.

Mozilla/5.0 (Windows NT 5.1; rv:6.0) Gecko/20110806 Firefox/6.0 SeaMonkey/2.3

Comment 2

6 years ago
Without looking too closely, appears that this has been an issue since SeaMonkey 2.1.

SeaMonkey 2.0.14 is not affected.
(Assignee)

Updated

6 years ago
Duplicate of this bug: 670509
(Assignee)

Comment 4

6 years ago
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.]

Comment 5

6 years ago
One option would be to stop using observer notifications to track windows.
(Assignee)

Comment 6

6 years ago
Maybe we should simply attach a trigger to the Bookmarks menu/popup (don't know if that's what Neil meant in comment 5).
(Assignee)

Comment 7

6 years ago
Created attachment 552794 [details] [diff] [review]
patch

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)
(Assignee)

Comment 8

6 years ago
Created attachment 552796 [details] [diff] [review]
patch v1a [Checkin: comments 10 and 13]

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 9

6 years ago
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+
(Assignee)

Comment 10

6 years ago
Comment on attachment 552796 [details] [diff] [review]
patch v1a [Checkin: comments 10 and 13]

http://hg.mozilla.org/comm-central/rev/b76dac043e0f
Attachment #552796 - Attachment description: patch v1a → patch v1a [Checkin: comment 10]
Attachment #552796 - Flags: approval-comm-aurora?
(Assignee)

Updated

6 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.5
(Assignee)

Comment 11

6 years ago
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-

Updated

6 years ago
Attachment #552796 - Flags: approval-comm-aurora? → approval-comm-aurora+
(Assignee)

Comment 13

6 years ago
Comment on attachment 552796 [details] [diff] [review]
patch v1a [Checkin: comments 10 and 13]

http://hg.mozilla.org/releases/comm-aurora/rev/271501c7d9a2
Attachment #552796 - Attachment description: patch v1a [Checkin: comment 10] → patch v1a [Checkin: comments 10 and 13]
(Assignee)

Updated

6 years ago
status-seamonkey2.3: --- → wontfix
status-seamonkey2.4: --- → fixed
status-seamonkey2.5: --- → fixed

Updated

6 years ago
Depends on: 682531
You need to log in before you can comment on or make changes to this bug.