564050 - "Save as PDF" should not fill in wrong titles for the Maemo file picker

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, when you do the "Save as PDF" thing on that page, the page title gets filled in, but it contains the ':' character, which doesn't seem to be accepted by the Maemo filesystem.
So I think Fennec shouldn't be adding those characters in the file picker to begin with (it should filter those characters out).

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch

This patch uses contentAreaUtils::getDefaultFileName to clean and validate the filename. It also adds some maemo-specific cleaning to handle the ":" character.

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #1)
> Created an attachment (id=443928) [details]
> patch
> 
> This patch uses contentAreaUtils::getDefaultFileName to clean and validate the
> filename. It also adds some maemo-specific cleaning to handle the ":"
> character.

Maybe we should clean up some others characters?
http://en.wikipedia.org/wiki/Filename#Reserved_characters_and_words

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #2)
> (In reply to comment #1)
> > Created an attachment (id=443928) [details] [details]
> > patch
> > 
> > This patch uses contentAreaUtils::getDefaultFileName to clean and validate the
> > filename. It also adds some maemo-specific cleaning to handle the ":"
> > character.
> 
> Maybe we should clean up some others characters?
> http://en.wikipedia.org/wiki/Filename#Reserved_characters_and_words

Reading at the list, not much more, maybe the "/"?

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #3)

> Reading at the list, not much more, maybe the "/"?

Already handled by validateFileName, called from getDefaultFileName

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 443928 [details] [diff] [review]
patch

Well, this is perfect.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed m-b:
http://hg.mozilla.org/mobile-browser/rev/26ab8381232b

pushed m-1.1:
http://hg.mozilla.org/releases/mobile-1.1/rev/0be52d1fed51

Comment 7

[Aakash Desai [:aakashd]]

verified FIXED on builds:

Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2.5pre) Gecko/20100507 Namoroka/3.6.5pre Fennec/1.1b2pre

and

Mozilla/5.0 (X11; U; Linux armv71; Nokia N900; en-US; rv:1.9.3a5pre) Gecko/20100507 Namoroka/3.7a5pre Fennec/2.0a1pre

Comment 8

[Aakash Desai [:aakashd]]

litmus testcase https://litmus.mozilla.org/show_test.cgi?id=11800 created to regression test this bug.