56574 - Imported IE Favorites in sidebar Bookmarks panel broken!

Status: VERIFIED FIXED

(SeaMonkey :: Bookmarks & History, defect, P1)

Description

[Ron Gavlin]

When I double-click on any of my Sidebar->Bookmarks->Imported IE Favorites, I
see this in the URL:

file:///C|/DOCUMENTS%20AND%20SETTINGS/RONALD%20L.%20GAVLIN/FAVORITES/HTML%20References.url

and this in the window:

[DEFAULT]
BASEURL=http://134.153.41.197/staff/gporter/htmlref.htm
[InternetShortcut]
URL=http://134.153.41.197/staff/gporter/htmlref.htm
Modified=B0BF134780D5BF01DE

This is a very recent regression. People who are trying to convert to
Mozilla/Netscape 6 will not appreciate this bug very much.

I've been seeing it in the MTrunk builds for a few days but today is the first
time I saw it in a NS6 build (Build ID: 2000101308).

BTW, the Imported IE Favorites work fine from the Bookmarks menu.

Comment 1

[Blake Ross]

I have a fix for this but it involves using OpenBookmarksURL() instead of 
sidebarOpenURL(), and I assume there's a reason why we're using the latter 
instead of the former, which the menu uses (I presume it's to fix some of those 
problems in composer/mailnews with sidebar urls not opening in the correct 
place...maybe rjc/ben can shed some light here?)

Comment 2

[shrirang khanzode]

comp : bkmarks

Comment 3

[Ron Gavlin]

This is a recent regression (within the last week) that should be fixed before
NS6 ships. My Sidebar is one of the NS6 features that might attract IE users. It
must import IE bookmarks correctly.

Comment 4

[Blake Ross]

rjc, can you say why we're using sidebarOpenURL() ?

Comment 5

[matt]

We are using this because bookmarks where opening in
the mail compose window.  This code is almost the same
as the search sidebar code.  looking at this bug right now.

Comment 6

[matt]

broke with this fix
http://bugzilla.mozilla.org/show_bug.cgi?id=48465

Comment 7

[don]

Marking this "rtm need info" ...

Comment 8

[matt]

I have a fix in hand.
Waiting for review.
For some reason ds = theRootNode.database; was
making the IE bookmarks barf.

Comment 9

[matt]

Attachment: patch for bm-panel.js

Comment 10

[Robert John Churchill]

r=rjc

Comment 11

[Alec Flett]

sr=alecf

Comment 12

[don]

OK, we have the fix approved and reviewed.

Comment 13

[Ron Gavlin]

Using trunk build 2000101804, now none of the "My Sidebar" Bookmarks work.
Double-clicking on any Bookmark is now simply ignored.

Comment 14

[Peter Trudelle]

PDT says rtm++, please land on branch ASAP, but be careful: if there are any
problems with this, we're inclined to back this out, along with the original
patch that caused this regression.

Comment 15

[Peter "jag" Annema]

It looks like you inadvertedly introduced some bustage on the trunk...
You're calling clicked with three params while it has two, and you left in a '}'
somewhere in openBookmarksURL causing a parsing error, resulting in none of the
functions being available, net result: double clicking a bookmark doesn't open
it.

I'll attach two patches, one whitespace fixed (be damned, you TABs), and one so
you can actually see what was wrong ;-)

Comment 16

[Peter "jag" Annema]

Attachment: [patch] -u -w Fixes bustage

Comment 17

[Peter "jag" Annema]

Attachment: [patch] -u Fixes bustage and whitespace

Comment 18

[matt]

ya when i checked into the trunk
i updatedd bm-panel.xul which was not supposed to happen.
I had a change in there i didn't want checked in which was really old.

The extra } was when it got patched wrong.
Got that one fixed.  In my branch i never had it.

I will fix the white space on the trunk but not on the 
branch.   PDT doesn't like that.

Comment 19

[Jacob Steenhagen]

OK, seems to be working in trunk build 2000102004 on Win2k.

Comment 20

[Asa Dotzler [:asa]]

This is Fixed on Mozilla trunk builds
win32 102004 NT 4
mac 102008 Mac OS9

Comment 21

[matt]

*** Bug 55700 has been marked as a duplicate of this bug. ***

Comment 22

[Ron Gavlin]

Is this going to go into the branch?

Comment 23

[matt]

fixed in branch
tested yesterday in trunk

Comment 24

[Claudius Gayle]

VERIFIED Fixed in 2000102509 branch builds.
adding vtrunk keyword

Comment 25

[Stephen Walker]

verified fixed with the 2000102604 trunk build on win2k.
removing vtrunk keyword.