Closed Bug 281343 Opened 20 years ago Closed 20 years ago

Cannot save web pages using very long filenames


(Core Graveyard :: File Handling, defect)

Windows XP
Not set


(Not tracked)



(Reporter: son.le0, Assigned: son.le0)



(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041109 Firefox/1.0 (MOOX M1)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041109 Firefox/1.0 (MOOX M1)

Follow on from bug 241864, comment #16

On Windows, the MAX_PATH for a filepath (drive letter + directory name + file
name) cannot be longer than 248 chars. nsWebBrowserPersist (and possibly other
places) fails to check against this limit and fails silently.

Reproducible: Always

Steps to Reproduce:
1. Open any webpage
2. Select Save Page As...
3. Enter the following filename
4. Click on save

Actual Results:  

Expected Results:  
Page saved or error message indicating save has failed.
Don't know which area this bug belongs, there was no embedding/components
selection. :)

Anyway, the problem is here:

Possible solutions are to truncate the filepath or fail and show an error message.
Version: unspecified → Trunk
Bug 129911 is also related...
Attached patch proposed patch v0 (obsolete) — Splinter Review
Display error message if save fails.
Christian, can you please have a look at this bug and patch for me?
Comment on attachment 174127 [details] [diff] [review]
proposed patch v0

you can not hardcode english messages.

Also, I am not sure that an unparented prompt is really a good thing.

Can't you just call SendErrorStatusChange, with an appropriate error code
(possibly a new one)?
Attachment #174127 - Flags: review-
Assignee: general → file-handling
Component: General → File Handling
Product: Mozilla Application Suite → Core
QA Contact: general → ian
Attached patch proposed patch v1 (obsolete) — Splinter Review
Use SendErrorStatusChange() instead, which works much better. Had to alter
nsLocalFileWin.cpp which is in xpcom (and added a missing break or two as

Not sure about the wording of the error messages. Does it need to be run by
anyone first? Translators?
Attachment #174127 - Attachment is obsolete: true
Attachment #174369 - Flags: review?(cbiesinger)
Comment on attachment 174369 [details] [diff] [review]
proposed patch v1

looks great to me, thanks!
I'm not sure that the text for the AlreadyExistsError is so good, since users
may not even know about a _files directory... I guess it's a rare case, so it
probably won't matter much.

r=biesi. please get review from darin or dougt for the xpcom change.
Attachment #174369 - Flags: review?(cbiesinger) → review+
Comment on attachment 174369 [details] [diff] [review]
proposed patch v1

dougt, asking for sr for xpcom change.

:bi, originally just had "file already exists" but found it was a bit
misleading as users might go looking for the wrong file. Thought the _files
might give them a hint.
Attachment #174369 - Flags: superreview?(dougt)
Flags: blocking-aviary1.1?
updated to match with new location
Attachment #174369 - Attachment is obsolete: true
Attachment #175682 - Flags: superreview?(dougt)
Attachment #175682 - Flags: review+
carried forward cbiesinger review+
Attachment #174369 - Flags: superreview?(dougt)
Attachment #175682 - Flags: superreview?(dougt) → superreview+
:bi, can you please check this in for me?

thanx in advance.
checked in for 1.8b2, thanks for the patch!

Checking in dom/locales/en-US/chrome/;
/cvsroot/mozilla/dom/locales/en-US/chrome/,v  <--
 nsWebBrowserPersist.propertiesnew revision: 1.2; previous revision: 1.1
Checking in embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp;
 <--  nsWebBrowserPersist.cpp
new revision: 1.106; previous revision: 1.105
Checking in xpcom/io/nsLocalFileWin.cpp;
/cvsroot/mozilla/xpcom/io/nsLocalFileWin.cpp,v  <--  nsLocalFileWin.cpp
new revision: 1.136; previous revision: 1.135
Assignee: file-handling → son.le0
Ever confirmed: true
Closed: 20 years ago
Resolution: --- → FIXED
+fileAlreadyExistsError=%S could not be saved, because a file already exists
with the same name as the '_files' directory.\n\nTry saving to a different location.
+fileNameTooLongError=%S could not be saved, because the file name was too
long.\n\nTry saving with a shorter file name.

nit: Didn't need the commas before "because".
Flags: blocking-aviary1.1?
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.