115991 - Save web page, complete can't cope with multiple files with the same name

Status: VERIFIED FIXED

(Core Graveyard :: File Handling, defect)

Description

[gavin long]

Consider :
A web page links two images with the same file name but in different directories.

If you save the page, including the images, then a "[pagename]_files" directory
is created, but you only end up with one of the images.  I'll attach a testcase
in a mo.

Comment 1

[gavin long]

Attachment: zipped-up testcase (use file paths when unzipping)

Unzip the testcase, using file paths.  You'll get an html file, and two images,
with the same file name, in two different sub-folders.

View the html in Moz, then try to save the page, including images.

You get a single "[filename]_files" directory, with only one image in it.

Comment 2

[gavin long]

Doh! no build details :  Win98SE, 2001121803.  Apologies for the spam...

Comment 3

[Adam Lock]

Note to self, nsWebBrowserPersist::MakeFilenameFromURI should keep a record of
what file names it has already generated and ensure that it doesn't duplicate them.

Comment 4

[Boris Zbarsky [:bzbarsky]]

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

Comment 5

[Boris Zbarsky [:bzbarsky]]

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

Comment 6

[Boris Zbarsky [:bzbarsky]]

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

Comment 7

[Boris Zbarsky [:bzbarsky]]

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

Comment 8

[Boris Zbarsky [:bzbarsky]]

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

Comment 9

[Adam Lock]

Attachment: Patch

This patch modifies the existing MakeFilenameFromURI() method so that URIs have
unique filenames. If two URIs are called image.gif, the second will become
image_2.gif.

I've also done a little cleanup on some naming conventions and replaced
PRPackedBool with PRBool in the structures.

Reviews?

Comment 10

[Adam Lock]

Attachment: Updated patch

Patch updated following feedback from Kathy.

Changes:

* Detabbed
* Naming convention for duplicates foo, foo_002, foo_003 etc.
* Naming convention for garbage links like "http://blah/;;;.gif" - _001, _002
etc.* Removed mFileCounter & mFrameCounter

Comment 11

[Kathleen :Brade]

in this block:
+            // Check for duplicates
+            if (mURIMap.Count() > 0)
+                mURIMap.Enumerate(EnumCheckForDuplicateFileNames, &dupData);

Why do you need to check mURIMap.Count() when the outer loop has already checked
that?

looks like there are still tabs in the header file?

Comment 12

[Adam Lock]

The outer look checks for mURIMap.Count() > 0 OR filename.isEmpty(). So the
enumeration code only needs to happen for the former case.

I will detab the header file.

Comment 13

[Kathleen :Brade]

Comment on attachment 72833 [details] [diff] [review]
Updated patch

doh! sorry I flaked on that (no wonder I didn't ask about that the first time
around! ;-) )
r=brade

Comment 14

[kinmoz]

Comment on attachment 72833 [details] [diff] [review]
Updated patch

sr=[e-mail]

Just fix the indentation of the code you added here:


 static PRBool PR_CALLBACK EnumCountURIsToPersist(
	 nsHashKey *aKey, void *aData, void* closure);
+	static PRBool PR_CALLBACK EnumCheckForDuplicateFileNames(
+		nsHashKey *aKey, void *aData, void* closure);

Comment 15

[Asa Dotzler [:asa]]

Comment on attachment 72833 [details] [diff] [review]
Updated patch

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 16

[Adam Lock]

Fix checked into the trunk

Comment 17

[Boris Zbarsky [:bzbarsky]]

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

Comment 18

[Michael Dunn]

to depstein for verification

Comment 19

[David Epstein]

verified, both images saved (one of images is renamed with prefix _002 because
it resides in the same folder). Mozilla 1.2b Mozilla/5.0 (Windows; U; WinNT4.0;
en-US; rv:1.2b) Gecko/20021026