150472 - Bookmark this group as tab will sometimes load www.bookmarks_groupmark.com

Status: RESOLVED WORKSFORME

(SeaMonkey :: Bookmarks & History, defect)

Description

[Travis Crump]

Bookmark this group as tab will sometimes try to load www.bookmarks_groupmark.com

Steps to reproduce:
1)  Load a bookmark group
2)  While the tabs are still loading(some of the tabs still say 'Loading' and
haven't even retrieved the title[once all the pages have a title this doesn't
seem to happen]), click on Bookmark this Group of Tabs.
3)  Mozilla attempts to load bookmarks_groupmark[followed by
www.bookmarks_groupmark.com due to another bug]

I was just able to reproduce this three times in a row.  This might be related
to the www.find.com fiasco, but at least this time no one owns the domain so you
just get an error message. This may also happen when only loading one site, but
is harder to reproduce since the title is usually retrieved faster then you can
get to the menu option or if network traffic is stopped than the error message
beats you.

Comment 1

[jag (Peter Annema)]

Simpler version: open a link in a new tab and try to bookmark the group, you'll
see the same effect (if you're fast enough, otherwise try ctrl-clicking a few
links with your settings so that ctrl-click opens in new tab and tabs load in
the background).

The problem for new tabs is that the .webNavigation object won't have been
constructed yet and is null, which we query for the currentURI, which results in
an exception and causes us to skip the event.preventBubble();. For existing tabs
(when you open a bookmark group it will try to reuse existing tabs) you'll get
the title, URL and charset for the page that was previously there.

You can actually see this latter effect in a "normal" browser window when you go
to a new page and press ctrl+shift+d while the page is still loading (again, you
have to be quick and/or find a page that loads slowly). Same for ctrl+d ("Add
Bookmark").

I can make sure that event.preventBubble() is called so you won't get this funny
dialog, that part is easy. The hard part is what to do about opening the
bookmark dialog:

1) Disable the menu item while pages are still loading
2) Postpone opening the dialog while pages are still loading
3) Skip all pages that are still loading
4) Skip only the pages whose browser's webNavigation is null

I think options 3 and 4 aren't really an option, the user won't know that not
everything got bookmarked, and/or perhaps "previous" pages got bookmarked,
unless/until they examine the bookmark group or try to open it.

Option 2 is kinda bad since there's no feedback to the user, unless we
explicitely throw up some kind of dialog explaining that (and why) we can't
bookmark this group until all the pages are done loading, with some cancel
button to cancel this action, and it'll go away when all pages are done loading
(non-trivial to implement).

That leaves us with option 1, which we might be able to implement for RTM.

Btw, this also affects the "File Bookmark..." menu items since they go through
the same code path (modulo defaulting to bookmarking the page of the currently
selected tab). Currently it just doesn't bring up the "add bookmark" dialog.
Since it will seem a little strange that one can't bookmark the page of the
currently selected tab because other pages are still loading, perhaps the best
solution here is to revert this code to simply bookmarking one page without the
convenience of the checkbox there to switch modes.

Hmmm, we might be able to make the "add bookmark" dialog somehow inform the user
that (s)he'll have to wait till all pages are done loading before a bookmark
group can be filed (which means this dialog will have to poll the browser window
for this information), and .... hmmm, I'll be quiet now.

Oh, we probably should also address the issue for "Add bookmark" and "File
Bookmark..." getting the info for the previous page if we're busy loading the
new page.

Comment 2

[Claudius Gayle]

Confirming based on jag's comments

Comment 3

[jag (Peter Annema)]

wfm