Cannot save web pages using very long filenames

RESOLVED FIXED

Status

RESOLVED FIXED
14 years ago
2 years ago

People

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

Tracking

Trunk
x86
Windows XP

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

14 years ago
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
1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678912345678
4. Click on save

Actual Results:  
Nothing.

Expected Results:  
Page saved or error message indicating save has failed.
(Assignee)

Comment 1

14 years ago
Don't know which area this bug belongs, there was no embedding/components
selection. :)

Anyway, the problem is here:

http://lxr.mozilla.org/mozilla/source/embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp#1586

Possible solutions are to truncate the filepath or fail and show an error message.
Version: unspecified → Trunk
(Assignee)

Comment 2

14 years ago
Bug 129911 is also related...
(Assignee)

Comment 3

14 years ago
Created attachment 174127 [details] [diff] [review]
proposed patch v0

Display error message if save fails.
(Assignee)

Comment 4

14 years ago
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
(Assignee)

Comment 6

14 years ago
Created attachment 174369 [details] [diff] [review]
proposed patch v1

Use SendErrorStatusChange() instead, which works much better. Had to alter
nsLocalFileWin.cpp which is in xpcom (and added a missing break or two as
well).

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+
(Assignee)

Comment 8

14 years ago
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)
(Assignee)

Updated

14 years ago
Flags: blocking-aviary1.1?
(Assignee)

Comment 9

14 years ago
Created attachment 175682 [details] [diff] [review]
new nsWebBrowserPersist.properties location

updated to match with new nsWebBrowserPersist.properties location
Attachment #174369 - Attachment is obsolete: true
Attachment #175682 - Flags: superreview?(dougt)
Attachment #175682 - Flags: review+
(Assignee)

Comment 10

14 years ago
carried forward cbiesinger review+
(Assignee)

Updated

14 years ago
Attachment #174369 - Flags: superreview?(dougt)

Updated

14 years ago
Attachment #175682 - Flags: superreview?(dougt) → superreview+
(Assignee)

Comment 11

14 years ago
: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/nsWebBrowserPersist.properties;
/cvsroot/mozilla/dom/locales/en-US/chrome/nsWebBrowserPersist.properties,v  <--
 nsWebBrowserPersist.propertiesnew revision: 1.2; previous revision: 1.1
done
Checking in embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp;
/cvsroot/mozilla/embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp,v
 <--  nsWebBrowserPersist.cpp
new revision: 1.106; previous revision: 1.105
done
Checking in xpcom/io/nsLocalFileWin.cpp;
/cvsroot/mozilla/xpcom/io/nsLocalFileWin.cpp,v  <--  nsLocalFileWin.cpp
new revision: 1.136; previous revision: 1.135
done
Assignee: file-handling → son.le0
Status: UNCONFIRMED → NEW
Ever confirmed: true
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED

Comment 13

14 years ago
+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".
(Assignee)

Updated

14 years ago
Flags: blocking-aviary1.1?
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.