87629 - bookmarks corrupted when filed

Status: VERIFIED FIXED

(SeaMonkey :: Bookmarks & History, defect)

Description

[Christopher Blizzard (:blizzard)]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.1+) Gecko/20010625
BuildID:    20010625

If you file the above listed bookmark it is saved in the bookmarks file corrupted

Reproducible: Always
Steps to Reproduce:
1. Go to the above url
2. Go to Bookmarks -> File Bookmark
3. Choose a folder to file it in
4. Open Bookmarks -> Manager Bookmarks
5. Look at your filed bookmark and note that it's damaged

Actual Results:  Bookmark comes out as:

file://http://bonsai.mozilla.org/cvsquery.cgi%3Ftreeid=default&module=all&branch=MOZILLA_0_9_2_BRANCH&branchtype=match&dir=&file=mozilla%252F&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=day&mindate=&maxdate=&cvsroot=%252Fcvsroot

Expected Results:  Should be:

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=MOZILLA_0_9_2_BRANCH&branchtype=match&dir=&file=mozilla%2F&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=day&mindate=&maxdate=&cvsroot=%2Fcvsroot

This is in the 0.9.2 branch.

Comment 1

[Christopher Blizzard (:blizzard)]

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

Comment 2

[Chase Tingley]

http://lxr.mozilla.org/mozilla/source/xpfe/components/bookmarks/resources/addBookmark.js#184

If I comment out this try block ("Check to see if the item is in a local
directory path..."; obviously don't comment out the var assignment...), the
problem appears to go away.

This is a pretty old code block (rev 1.2, january, according to lxr), so maybe
something is no longer throwing the expected exception?

Off to figure out what's supposed to be going on.

Comment 3

[Chase Tingley]

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

Comment 4

[Chase Tingley]

cc'ing dougt, because I think this problem stems from his big url checkin on the
21st.

When I file a bookmark, I see an assertion at nsLocalFile::GetURL(): "Cannot
tell if this is a directory."  Looking at the code (nsLocalFileUnix.cpp in my
case).  I notice that *aURL is still set to the modified (file:// prepended)
value even if the directory lookup fails.  If i move the assignment inside the
if block, the problem appears to go away.

I'm sort of using the force here, but I'll put together a patch to show you what
I mean.  Assuming this is an appropriate change, it appears that it's needed in
nsLocalFile*.cpp for each platform (excluding Common).

Comment 5

[Chase Tingley]

Hmm, forget it.  After rereading the code, I've concluded that even though that
works, it is pretty clearly not be the correct solution.  Sorry for spam.

Comment 6

[Chris Hiner]

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

Comment 7

[kent dorsey]

The following sequence of commands can be used to consistently reproduce this
bug under Windows 2000 Server on one of my machines.  Hope this helps (and works
for others).

1. Start Mozilla (my settings start with a mail window).
2. Clear memory and disk caches.
2.a. Select "Edit / Preferences / Advanced / Cache / Clear Memory Cache"
2.b. Select "Edit / Preferences / Advanced / Cache / Clear Disk Cache"
3. Open a browser window.
Note: Steps 2 and 3 can be reversed; browser can be set to open with blank page
or live web page.
4. Select "Debug / Viewer Demos / #9 Frames" (loads from resource)
5. Select "Debug / XBL Demos / #1 Technicolor DIV" (loads from www.mozilla.org)
6. The page will not completely load.  Incomplete load appears related to frames?
7. Go to non-www.mozzila.org web site.
8. Repeat step 2 to clear caches.
9. Repeat steps 4 and 5 to reproduce again, ad nauseum.

Did not test further for other possibilities.  My cache settings are 4096
memory, 5000 disk, Check every time.

Comment 8

[kent dorsey]

Sorry, the comments posted previously were meant for bug #83175

Comment 9

[Nick Lewycky]

I'm not sure if this is the same bug or if I should file a new one, but I have a
gopher directory bookmarked and when I select a file from inside that directory
(using Bookmarks from the Personal Toolbar) it concatenates the two URLs
together resulting in a url with two "gopher://"es in it.

Comment 10

[chris hofmann]

ben, alecf, dougt: any ideas on the fix for this.  we need a fix badly 
to wrap up mozilla 0.9.2 get on with more work on that branch. 

Comment 11

[Doug Turner (:dougt)]

 Nick Lewycky - that is a seperate bug.  Please file it, although I think that I
had see a dup....

Comment 12

[Doug Turner (:dougt)]

I am rebuilding right now.  I can take a look at this when my build is done.  

By inspection, I think tingley was on the right track.  I wonder what
|gFld_URL.value| is when |initWithUnicodePath| is called.  I bet it is
"http://bonsai.mozilla.org/......"  Know more soon....

Comment 13

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: patch to fix by calling exists() instead of relying on exception to be thrown

Comment 14

[Doug Turner (:dougt)]

Ben, PERFECT!  that worked.

r/sr=

I can check in this as soon as I get a approval and another r=.  

Comment 15

[Alec Flett]

ok, seems reasonable
r/sr=alecf

Comment 16

[Christopher Blizzard (:blizzard)]

a=blizzard on behalf of drivers for 0.9.2

Comment 17

[Christopher Blizzard (:blizzard)]

Checked into the 0.9.2 branch.  This still needs to be checked into the trunk
but it's closed right now.

Comment 18

[Doug Turner (:dougt)]

Checking in addBookmark.js;
/cvsroot/mozilla/xpfe/components/bookmarks/resources/addBookmark.js,v  <--  
addBookmark.js

new revision: 1.7; previous revision: 1.6
done

Comment 19

[Claudius Gayle]

VERIFIED Fixed for 2001072405 Branch and Trunk builds