Closed Bug 115991 Opened 19 years ago Closed 19 years ago

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

Categories

(Core Graveyard :: File Handling, defect)

x86
Windows 98
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.0

People

(Reporter: gav, Assigned: adamlock)

References

Details

(Keywords: topembed)

Attachments

(2 files, 1 obsolete file)

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.
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.
Doh! no build details :  Win98SE, 2001121803.  Apologies for the spam...
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.
Blocks: 115634
Target Milestone: --- → mozilla0.9.9
*** Bug 122600 has been marked as a duplicate of this bug. ***
*** Bug 124116 has been marked as a duplicate of this bug. ***
*** Bug 120017 has been marked as a duplicate of this bug. ***
*** Bug 125719 has been marked as a duplicate of this bug. ***
Target Milestone: mozilla0.9.9 → mozilla1.0
*** Bug 127542 has been marked as a duplicate of this bug. ***
Keywords: topembed
Attached patch Patch (obsolete) — Splinter Review
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?
Attached patch Updated patchSplinter Review
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
Attachment #72804 - Attachment is obsolete: true
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?
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 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
Attachment #72833 - Flags: review+
Comment on attachment 72833 [details] [diff] [review]
Updated patch

sr=kin@netscape.com

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);
Attachment #72833 - Flags: superreview+
Comment on attachment 72833 [details] [diff] [review]
Updated patch

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #72833 - Flags: approval+
Fix checked into the trunk
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
QA Contact: sairuh → mdunn
*** Bug 134303 has been marked as a duplicate of this bug. ***
to depstein for verification
QA Contact: mdunn → depstein
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
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.