209097 - Default mailto form subject should be localizable

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Ansgar Schrage]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de-AT; rv:1.4b) Gecko/20030507
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de-AT; rv:1.4b) Gecko/20030507

When you use Mozilla to post a form via email, the subject line of the created
email has an English text: 'Form Post From Mozilla'
I suggest to replace it (if possible) with a German text. What about 'Formular
wurde über Mozilla versendet'.

Reproducible: Always

Steps to Reproduce:
1. Click on the send button of a form that is sent via Email

Comment 1

[Gunnar Kaestle]

Kannst Du eine Beispiel-URL nennen, wo dieser Effekt auftritt?

Comment 2

[Ansgar Schrage]

An example page where this effect can be seen is here
http://www.atelier-scheuch.de/email_formular.html 

Comment 3

[Robert Kaiser]

This is not localizable currently, it's a hardcoded subject string that is used
if there's no subject set in the mailto: form.

see
http://lxr.mozilla.org/mozilla/source/content/html/content/src/nsFormSubmission.cpp#428

(this was introduced by bug 61893)

If you think it's still important that the default subject should get
translated, then we should morph this bug into a "Default mailto form subject
should be localizable" bug in Browser / Form Submission component with l12y
keyword set.

Ansgar, what do you think (as you're the original poster)?

Comment 4

[Ansgar Schrage]

In my opinion this new bug should be created. It is not a big issue, but when a
complete translation is the goal, this should be part of it. Robert will you
create the new bug or should it done by me?

Comment 5

[Robert Kaiser]

We don't need a new bug, we can just use this one, as it already holds all the
needed information, and localizations will adapt automatically when it's changed
in the main tree.

Moving per Comment #3

Comment 6

[Robert Kaiser]

reassigning. see comments #3 to #5.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Note that layout/html/forms/src/HtmlForm.properties is probably a good place to
put the string...

Comment 8

[Ajoy Bhatia]

Trying to fix this bug - my first. In my local tree, I added
the DefaultSubject string to layout/html/forms/src/HtmlForm.properties,
as Boris suggested. To retrieve the string where it is used
at: content/html/content/src/nsFormSubmission.cpp, I made a
call to nsFormControlHelper::GetLocalizedString method declared
in layout/html/forms/src/nsFormControlHelper.h after doing
a #include of this header. This is the same method used to
retrieve another string from HtmlForm.properties in the file
layout/html/forms/src/nsFileControlFrame.cpp

The code did not compile because the nsFormControlHelper.h
file is not found when compiling nsFormSubmission.cpp . This
file is in the same dir as nsFileControlFrame.cpp - the other
place where this method is called - which is why it might have
been found when that file is compiled.

Can the .h be accessed from the .cpp file mentioned above? Or
perhaps, I should look for another place to put the DefaultSubject
string?

Comment 9

[Axel Hecht]

Let's just add that I'm not convinced that putting a content string into layout.
But then bsmedberg might know where he's going to move files like this anyway.
I see good reasons for layout headers not working in content, so that'd be fine.
See what the called function does and mimic it. Or grep in the content sources
if there is an equivalent helper in content. (Can you smell somebody not wanting
content-layout to intermongle even further?)

Comment 10

[[e-mail]]

While you're fixing this please remove the extra & at the end of the subject.

Comment 11

[Ajoy Bhatia]

Thought I'd found a good place to put this string - in include/allxpstr.h
and then use XP_GetString() to fetch it, as specified in:
http://www.mozilla.org/docs/refList/i18n/allxpstr-h.html

Sadly, it looks like this is outdated documentation. Couldn't find this
file in the source tree.

Comment 12

[Mike Kaply [:mkaply]]

Yes, that's very old documentation.

I don't see any good properties file under content for this either.

Logically, HTMLForm.properties makes sense, because this is form related...

Comment 13

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Is this being worked on?  If not, I'll take it.

Comment 14

[Christian :Biesinger (don't email me, ping me on IRC)]

So... why should this be fixed? The language of the recipient may well not be
the language of the user's browser. This means that the recipient may not be
able to tell that a mail is indeed the result of a form post. It also means that
it is not possible to recognize a form post just by looking at its subject.

Comment 15

[Mike Kaply [:mkaply]]

What do IE, Opera, etc. do?

Comment 16

[Micha³ Piaskowski]

IE doesn't set any subject (empty subject line).

Comment 17

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

(In reply to comment #14)
> So... why should this be fixed? The language of the recipient may well not be
> the language of the user's browser. This means that the recipient may not be
> able to tell that a mail is indeed the result of a form post.
They could/should have set a subject.  The way it is right now, every browser is
going to produce a different subject (or in the case of MSIE, every user is
going to type a different subject)

> It also means that it is not possible to recognize a form post just by looking
at its subject.
We're apparently the only ones who set a default subject anyway, so it doesn't
seem like we'd be harming many people's heuristics.

Comment 18

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

->me, patch in a bit

Comment 19

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: Patch

Comment 20

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 168193 [details] [diff] [review]
Patch

dom/locales/en-US/chrome/layout/HtmlForm.properties
\ No newline at end of file

please fix ;)

Comment 21

[Johnny Stenback (:jst)]

Comment on attachment 168193 [details] [diff] [review]
Patch

- In dom/locales/en-US/chrome/layout/HtmlForm.properties:

+DefaultFormSubject=Form Post from Mozilla
\ No newline at end of file

Add a newline at the end of the file.

Comment 22

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: to check in

Comment 23

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Comment on attachment 168235 [details] [diff] [review]
to check in

Err, sorry.

Comment 24

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: to check in

Comment 25

[[e-mail]]

Comment on attachment 168236 [details] [diff] [review]
to check in

>-    aPath.AppendLiteral("subject=Form%20Post%20From%20Mozilla&");
The old string was escaped. I don't see the new string being escaped...

Comment 26

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: Patch that escapes the strings

Comment 27

[[e-mail]]

Comment on attachment 168551 [details] [diff] [review]
Patch that escapes the strings

>+    rv = bundleService->CreateBundle(
>+                                     "chrome://communicator/locale/layout/HtmlForm.properties",
>+                                     getter_AddRefs(bundle));
Nit: only needs to be two lines.

Comment 28

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: to check in

Removed extra blank line

Comment 29

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

checked in by db48x

Comment 30

[Boris Zbarsky [:bzbarsky]]

Is there a reason that this didn't use the nsContentUtils methods for this sort of thing?

Comment 31

[Robert O'Callahan (:roc) (email my personal email if necessary)]

No. Ctho?

Comment 32

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

(In reply to comment #30)
> Is there a reason that this didn't use the nsContentUtils methods for this
sort of thing?

(In reply to comment #31)
> No. Ctho?

I'd never heard of it, and nobody mentioned it while I was writing the patch. 
I'm redoing it now.

Comment 33

[Robert O'Callahan (:roc) (email my personal email if necessary)]

great, thanks!

Comment 34

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: use nsContentUtils::GetLocalizedString

Comment 35

[[e-mail]]

There's a case for making subjectStr an nsAutoString, and an IMHO better case
for making nsContentUtils::Get/FormatLocalizedString take an nsXPIDLString&

Comment 36

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

checked in by timeless