Closed Bug 314222 Opened 16 years ago Closed 16 years ago

If link target URL has non-ASCII char that is not encoded by UTF-8, the default file name is always escaped at "Save Link As..."

Categories

(Firefox :: File Handling, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 2

People

(Reporter: masayuki, Assigned: masayuki)

References

()

Details

(4 keywords)

Attachments

(2 files, 3 obsolete files)

If link target URL has non-ASCII char that is not encoded by UTF-8, the default file name is always escaped at "Save Link As..."

The test case is here.
http://bugzilla.mozilla.gr.jp/attachment.cgi?id=2931&action=view

You may look Japanese default file name only on the "href has encoded by UTF-8".
On other links, you can look the escaped default file name.
Attached patch Patch rv1.0 (obsolete) — Splinter Review
aDocument is always null in initFileInfo if it's called by "Save Link As...".
We should use current document charset instead of aDocument's charset if aDocument is null.
Attachment #201149 - Flags: review?(mconnor)
Status: NEW → ASSIGNED
Keywords: regression
Comment on attachment 201149 [details] [diff] [review]
Patch rv1.0

Oops. This patch doesn't work at Alt + Click.
Attachment #201149 - Flags: review?(mconnor) → review-
Attached patch Patch rv1.1 (obsolete) — Splinter Review
Attachment #201149 - Attachment is obsolete: true
Attachment #201154 - Flags: review?(mconnor)
This bug is major. Because on Web Mail Service, the attached file has non-ASCII name, this bug is occured. This bug is very serious i18n bug for marketing.
But I'm waiting biesi's review in bug 314231. We should check in same patch to both xpfe and toolkit.
Severity: minor → major
Depends on: 314231
Priority: -- → P1
Comment on attachment 201154 [details] [diff] [review]
Patch rv1.1

>+ * @param aReferrer the referrer URI object (not URL string) to use, or null
>+ *        if no referrer should be sent.

this comment is bogus, since we're simply passing the referrer to extract the charset, just use something like this:

* @param aReferrer An nsIURI object for the referrer document, null if there is no referrer
Attachment #201154 - Flags: review?(mconnor) → review+
Thank you. But I wait XPFE's review too.
I'll check-in this patch and bug 314231's one in same time.
Flags: blocking1.8.1?
Flags: blocking1.8.0.1?
Attached patch Patch rv2.0 (obsolete) — Splinter Review
Updtated by biesi's review in bug 314231.
Attachment #201154 - Attachment is obsolete: true
Attachment #206232 - Flags: review+
Attachment #206232 - Attachment is obsolete: true
Attachment #206519 - Flags: review+
Attachment #206519 - Flags: approval1.8.0.1?
checked-in.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Mozilla Japan needs to be fixed on 1.8 branch too.
This bug is very serious for Web Mail Service. See comment 4.
Attachment #206519 - Flags: approval1.8.1?
Comment on attachment 206519 [details] [diff] [review]
Patch for check-in

I don't think that we should be changing the signature of initFileInfo() on the stable branch.
benjamin:

But this is very serious regression from 1.0.x. See comment 4.
Masayuki, that's fine, but we've made a commitment not to change APIs (even non-frozen ones) on the stable branch. We could add a *new* function signature or perhaps add the charset as an optional argument on the end of this function signature without changing the API.
Umm... Should I recreate new patch only for 1.8.0 branch that has another name function? (e.g., initFileInfo2())

# We can use current patch on 1.8 branch, right?
Comment on attachment 206519 [details] [diff] [review]
Patch for check-in

Yes, the current patch is fine for 1.8.1. I'll bring up the 1.8.0.x issue in the drivers meeting and get back to you.
Attachment #206519 - Flags: approval1.8.1? → approval1.8.1+
Thank you Benjamin. Please check bug 314231 too.
Could this be rewritten to put the charset parameter at the end? Then it could be treated optionally and not break anyone else using it.
Attachment #206519 - Flags: approval1.8.0.1?
Attachment #207798 - Flags: review+
Attachment #207798 - Flags: approval1.8.0.1?
Daniel:

Please check the new patch.
Attachment #207798 - Flags: review+ → review?(mconnor)
Flags: blocking1.8.1?
Flags: blocking1.8.1+
Flags: blocking1.8.0.1?
Flags: blocking1.8.0.1+
Whiteboard: need r=mconnor
Status: RESOLVED → VERIFIED
Attachment #207798 - Flags: review?(mconnor) → review+
Comment on attachment 207798 [details] [diff] [review]
Patch for 1.8.0.x

a=dveditz for drivers
Attachment #207798 - Flags: approval1.8.0.1? → approval1.8.0.1+
Thank you everyone!
Keywords: fixed1.8.0.1
Whiteboard: need r=mconnor
I just pooped my pants.. please help me clean myself up. I believe that it is all mozilla/firefox's fault cause I was doing some reverse-engineering with the beta browser, to try to make it toilet/thundermug compatible, and all of a sudden I lost complete control of my bowels with the brain-band synch hookup to the laptop and the toilet.  My poop was supposed to go straight to the toilet, but instead it coated all two layers of my below-waist clothing -- plus down both of my pant legs.  Obviousely, I had to spoon all of the brown lava into my mouth with a serving spoon until I could fit my pants and shorts back on without it dripping all over the floor, ( I wouldn't dare getting kah-kah on my bearskin rug ).  I am not really complaining -- as I should have taken extra precautions (like maybe connecting my bowel oriface to the toilet/thundermug), before trying a teleport of such nature -- however I am offering my experience as information to you for the purpose of improving mozilla, firefox.  Take it easy ya'll.. 

Mary McGlachenflachensteinbruner
You need to log in before you can comment on or make changes to this bug.