159377 - tab-group bookmark does nothing when no windows are open

Status: VERIFIED FIXED

(SeaMonkey :: Bookmarks & History, defect)

Description

[Scott Collins]

If no window is open, and I select a (single page) bookmark from the bookmark
menu, a new window opens and goes to the selected page.  If no window is open
and I select a bookmark representing a group of pages, nothing happens.  No
window opens.  100% reproducible on my MacOS X machine running 1.1b.  I imagine
this will not be reproducible on system where you cannot choose a bookmark
unless a window is open.

Comment 1

[Edward MacGillivray]

I have found that in both Linux and Windows, and in both versions 1.2.1 and 1.3
a similar (if not tha same) problem occurring.  I don't know much about clicking
the bookmark (or group) without a window being open.  However, if I have a
Mozilla window open and right click on a group of tabs that is on my Personal
Toolbar and then select "Open in New Window", then I actually get a new window
that is like the Bookmark Manager but it only shows the bookmarks of the tab
group that I wanted open in a new window.

What I just said about where I have found this bug is more accurately
represented by the following table:

        Windows    Linux
1.2.1     yes        yes
1.3       yes        untested

Normally, if I right click on a bookmark on my Personal Toolbar and select "Open
in New Window", then a new window opens and the bookmark is loaded in that
window.  This is what I expected to happen for the tab group too.

Also, if I right-click on the default action on a tab group, which is "Expand",
nothing happens.  This is odd because when I just left-click on the tab group it
does "expand".

All of what I have described in this message also applies to the context menu
(right-click) when using the Bookmark Manager.  Although "Expand" takes on a new
meaning, that being of expand tree, and when expand-ed the item changes to
"Collapse".

It appears to me that the "group of tabs" have not gotten as much attension as
regular bookmarks.  Or perhaps there is some confusion between whether "group of
tabs" is a folder or branch, OR whether it is actually a leaf.  The problem is
that is a hybrid of both.

mac

Comment 2

[Andreas Kunz]

This appears on all platforms and is because getTopWin() in
http://lxr.mozilla.org/seamonkey/source/xpfe/components/bookmarks/resources/bookmarks.js#575
does return null when no browser window is open and the script aborts there.

Edward: what you say is clearly different from this bug and does not belong here.

Comment 3

[Andreas Kunz]

Attachment: patch -uw for easier review (functionally identical to next one)

The real patch has whitespace changes all over because indentation has to be
changed. So the real patch looks much bigger than actual changes are.
This one is easier to understand.

Comment 4

[Andreas Kunz]

Attachment: Real patch: open tabgroup when no window available, plus some extras

This patch prevents openGroupBookmark from failing when there is no browser
window open by opening a new one with the tab group.

Additionally, "Open Group in new window" was "not supported" before. It is
actually just what I'm doing in this bug's special case, only always. So I
implemented it along the way without having to change much.

And I found three obviously unused variables, this patch removes them.

Comment 5

[Andreas Kunz]

Comment on attachment 139140 [details] [diff] [review]
Real patch: open tabgroup when no window available, plus some extras

Requesting pch's opinion on this approach.

Comment 6

[Pierre Chanial]

Comment on attachment 139140 [details] [diff] [review]
Real patch: open tabgroup when no window available, plus some extras

-      var w	    = getTopWin();
indentation

+      var browser  = w.document.getElementById("content");
use w.getBrowser() instead.

r=me with these changes, you will be able to easily add a context menu to load
a groupmark in a new window.

Comment 7

[Andreas Kunz]

Attachment: revised patch plus open bookmarks group in new window

Review comments addressed, adds a context menu entry for bookmarks groups to
open them in a new window.

Comment 8

[Andreas Kunz]

Comment on attachment 139408 [details] [diff] [review]
revised patch plus open bookmarks group in new window

Again requesting r= from pch because of two small changes.
First one is the context menu entry, second: for making "Open in new Window"
actually open in a _new_ window in all cases, I cannot use getTopWin() anymore,
but have to use openDialog - just as the function openOneBookmark already does.

Comment 9

[Pierre Chanial]

Comment on attachment 139408 [details] [diff] [review]
revised patch plus open bookmarks group in new window

It would be good to have that in Firebird, also.

Comment 10

[Andreas Kunz]

Comment on attachment 139408 [details] [diff] [review]
revised patch plus open bookmarks group in new window

Requesting sr= from alecf.

pch: my tree does not include Firebird, so all I could possibly do is write the
patch, but not test it.

Comment 11

[Alec Flett]

Comment on attachment 139408 [details] [diff] [review]
revised patch plus open bookmarks group in new window

sr=alecf

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

Checking in xpfe/components/bookmarks/resources/bookmarks.js;
/cvsroot/mozilla/xpfe/components/bookmarks/resources/bookmarks.js,v  <-- 
bookmarks.js
new revision: 1.126; previous revision: 1.125
done

Comment 13

[Andreas Kunz]

v