Empty "Content-Language:" header line produced if no dictionary is installed

RESOLVED FIXED in Thunderbird 53.0

Status

MailNews Core
MIME
RESOLVED FIXED
5 months ago
5 months ago

People

(Reporter: Jorg K (GMT+2), Assigned: Jorg K (GMT+2))

Tracking

Trunk
Thunderbird 53.0

Thunderbird Tracking Flags

(thunderbird50 wontfix, thunderbird51 fixed, thunderbird52 fixed, thunderbird53 fixed, seamonkey2.46 unaffected, seamonkey2.47 fixed, seamonkey2.48 fixed, seamonkey2.49 fixed, seamonkey2.50 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 months ago
See bug 1169184 comment #32 and two further comments.

Updated

5 months ago
status-seamonkey2.46: --- → unaffected
status-seamonkey2.47: --- → affected
status-seamonkey2.48: --- → affected
status-seamonkey2.49: --- → affected
status-seamonkey2.50: --- → affected
(Assignee)

Comment 1

5 months ago
Created attachment 8816796 [details] [diff] [review]
1322054-empty-content-language.patch (v1).

We had complaints from SM that an empty header was written since they haven't ported bug 1169184 yet, see bug 1289493.

We would write an empty header if the comp-field were an empty string, which I think can happen if there is no dictionary installed. Hard to test, since we always build with the en-US dictionary. Anyway, I tested this by hacking contentLanguage = "" in that hunk.

This additional check does no harm. Note that the code in nsMsgCompUtils.cpp wasn't necessary, the header gets copied over in finalHeaders->AddAllHeaders(fields) a few lines up.

The associated test is
https://dxr.mozilla.org/comm-central/rev/79a8ad3930a66444320f89102a45102ccf51186c/mail/test/mozmill/composition/test-drafts.js#203
and that still passes.
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #8816796 - Flags: review?(acelists)
Attachment #8816796 - Flags: feedback?(frgrahl)

Comment 2

5 months ago
Comment on attachment 8816796 [details] [diff] [review]
1322054-empty-content-language.patch (v1).

Review of attachment 8816796 [details] [diff] [review]:
-----------------------------------------------------------------

::: mailnews/compose/src/nsMsgSend.cpp
@@ +2780,5 @@
> +  char* contentLanguage = nullptr;
> +  fields->GetContentLanguage(&contentLanguage);
> +  if (contentLanguage && !*contentLanguage)
> +    contentLanguage = nullptr; // Delete the field.
> +  mCompFields->SetContentLanguage(contentLanguage);

Doesn't SetContentLanguage() do the same at https://dxr.mozilla.org/comm-central/rev/79a8ad3930a66444320f89102a45102ccf51186c/mailnews/compose/src/nsMsgCompFields.cpp#105 (it calls SetAsciiHeader)?

Comment 3

5 months ago
Comment on attachment 8816796 [details] [diff] [review]
1322054-empty-content-language.patch (v1).

Just built Seamonkey 2.50a1 and posted to mozilla.support.seamonkey with it. The empty Content-Language field is gone so an f+ and a big thanks from me.

If this works for TB too please push it down to c-b. I can push it to c-r.
Attachment #8816796 - Flags: feedback?(frgrahl) → feedback+
(Assignee)

Comment 4

5 months ago
(In reply to Frank-Rainer Grahl from comment #3)
> If this works for TB too please push it down to c-b. I can push it to c-r.
Haha, before I can push anything, I need to get it past Acemen who is already questioning the solution ;-)


(In reply to :aceman from comment #2)
> Doesn't SetContentLanguage() do the same at ...
Hmm, looks like it. Good catch! Well, perhaps we get away be just removing the other faulty hunk. Right now I need to check Magnus' security bug, but I'll get back to this later tonight.

Comment 5

5 months ago
> Haha, before I can push anything, I need to get it past Acemen 
> who is already questioning the solution ;-)

Sure no rush. Not all servers react allergic to it or it would have been discovered earlier. Just came up in the newsgroups today. Just holler if you want me to test a new patch.

FRG
(Assignee)

Comment 6

5 months ago
Created attachment 8816803 [details] [diff] [review]
1322054-empty-content-language.patch (v2).

Holler!!
Attachment #8816796 - Attachment is obsolete: true
Attachment #8816796 - Flags: review?(acelists)
Attachment #8816803 - Flags: feedback?(frgrahl)

Comment 7

5 months ago
Comment on attachment 8816803 [details] [diff] [review]
1322054-empty-content-language.patch (v2).

Works fine too.
Attachment #8816803 - Flags: feedback?(frgrahl) → feedback+
(Assignee)

Updated

5 months ago
Attachment #8816803 - Flags: review?(acelists)

Comment 8

5 months ago
Comment on attachment 8816803 [details] [diff] [review]
1322054-empty-content-language.patch (v2).

Review of attachment 8816803 [details] [diff] [review]:
-----------------------------------------------------------------

So the SetRawHeader() doesn't clear out headers with empty values.
Attachment #8816803 - Flags: review?(acelists) → review+
(Assignee)

Comment 9

5 months ago
C-C (TB 53): https://hg.mozilla.org/comm-central/rev/85dd34e18d173d05872889f4b9bb3691b61be6f5
C-A (TB 52): https://hg.mozilla.org/releases/comm-aurora/rev/99f2496ef9788d0aa028fb87f98f2da336db1fe2
C-B (TB 51): https://hg.mozilla.org/releases/comm-beta/rev/62c75cca01eb04c698cec4ee512316f316d3b0c4
(not part of TB 51 beta1 build2)

FRG, please land on C-R if you want.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
status-seamonkey2.48: affected → fixed
status-seamonkey2.49: affected → fixed
status-seamonkey2.50: affected → fixed
status-thunderbird50: --- → wontfix
status-thunderbird51: --- → fixed
status-thunderbird52: --- → fixed
status-thunderbird53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 53.0
(Assignee)

Comment 10

5 months ago
Comment on attachment 8816803 [details] [diff] [review]
1322054-empty-content-language.patch (v2).

Landed/approved for all branches. Minor tweak to remove some wrong code that caused problems ;-(
Attachment #8816803 - Flags: approval-comm-release+
Attachment #8816803 - Flags: approval-comm-beta+
Attachment #8816803 - Flags: approval-comm-aurora+

Updated

5 months ago
Summary: Empty Content-Language: header produced when no dictionary is installed → Empty "Content-Language: header" produced when no dictionary is installed

Comment 11

5 months ago
Although Dictionaries are installed for German, French, en-US:
REPRODUCIBLE with Server-Installation of  unofficial (by FRG) en-US SeaMonkey 2.50a1 (NT 6.1; Win64; x64; rv:53.0) Gecko/20100101 Firefox/53.0 Build 20161127221846  (Default Classic Theme) on German WIN7 64bit
Summary: Empty "Content-Language: header" produced when no dictionary is installed → Empty "Content-Language:" header line produced if no dictionary is installed
(Assignee)

Comment 12

5 months ago
(In reply to Rainer Bielefeld from comment #11)
> Although Dictionaries are installed for German, French, en-US:
Sure. Because SM is missing bug 1169184 / bug 1289493.
In TB it would only happen if no dictionary were installed, or perhaps not at all, I didn't test.

We fixed this as a favour for FRG, so never mind the summary. I didn't want to put:
  Fix in mailnews/ to cater for non-ported bug 1169184, see SM bug 1289493.

Comment 13

5 months ago
Thanks.

https://hg.mozilla.org/releases/comm-release/rev/4da63141cb902adf573947586a2e52915ad4ebb1

At least SeaMonkey de ships without a dictionary initially so it might do some good even the bug is ported.

Updated

5 months ago
status-seamonkey2.47: affected → fixed

Updated

5 months ago
Duplicate of this bug: 1322048
You need to log in before you can comment on or make changes to this bug.