Unable to add accounts when a DWM registry key is missing.

RESOLVED FIXED in Thunderbird 37.0

Status

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: apoliak, Assigned: mkmelin)

Tracking

31 Branch
Thunderbird 37.0
x86_64
Windows 8.1

Thunderbird Tracking Flags

(thunderbird36 fixed, thunderbird_esr3135+ fixed)

Details

Attachments

(2 attachments)

Reported by someone else in the support site: https://support.mozilla.org/en-US/questions/1015941

I had a user affected by this issue (Windows 8.1 Pro [fully patched], en-US, x86_64) and having them follow the instruction at https://support.mozilla.org/en-US/questions/1015941#answer-660970 addressed the issue.

However, the defect in Thunderbird is still in place an occurs within these two lines:

let windowFrameColor = WindowsRegistry.readRegKey(Ci.nsIWindowsRegKey.ROOT_KEY_CURRENT_USER,
 "Software\\Microsoft\\Windows\\DWM", "ColorizationColor");

and

let windowFrameColorHex = windowFrameColor.toString(16);

The second line uses the output of the first line without verifying that the first line has delivered correct data.

There's two ways this could be addressed:

Add exception handling around the existing code block to abort in a more graceful manner.

Modify the code to ensure that the windowFrameColor variable has acceptable contents before attempting to use it.
Sync up with firefox version of Windows8WindowFrameColor.jsm
Assignee: nobody → mkmelin+mozilla
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8532806 - Flags: review?(richard.marti)
Comment on attachment 8532806 [details] [diff] [review]
bug1107902_ColorizationColor.patch

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

Looks good. It's completely equal to the browser file, except the nit.

::: mail/base/modules/Windows8WindowFrameColor.jsm
@@ +46,3 @@
>    },
>  };
> +

nit: a LF to much.
Attachment #8532806 - Flags: review?(richard.marti) → review+
(In reply to Richard Marti (:Paenglab) from comment #2)
> nit: a LF to much.

Source files should preferably have a blank line last, no? (Many bash utils expect it so you can get strange results if you mod files with scripts and there's actually something of importance last. Like an xml files closing tag...)
Yes, but this file has with this patch two LF's at the end.
Ah yes...

https://hg.mozilla.org/comm-central/rev/03a14c5d78a3 -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 37.0
Comment on attachment 8532806 [details] [diff] [review]
bug1107902_ColorizationColor.patch

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: 
Testing completed (on c-c, etc.): 
Risk to taking this patch (and alternatives if risky):
Attachment #8532806 - Flags: approval-comm-esr31?
Attachment #8532806 - Flags: approval-comm-beta?
Attachment #8532806 - Flags: approval-comm-aurora?
For ESR we should just land the minimal fix:

     if (!windowFrameColor) {
       // Seems to be the default color (hardcoded because of bug 1065998/1107902)
       return [158, 158, 158];
     }
Comment on attachment 8532806 [details] [diff] [review]
bug1107902_ColorizationColor.patch

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

Please can someone create and get reviewed, the minimal fix that Magnus suggests.
Attachment #8532806 - Flags: approval-comm-esr31?
Attachment #8532806 - Flags: approval-comm-esr31-
Attachment #8532806 - Flags: approval-comm-beta?
Attachment #8532806 - Flags: approval-comm-beta+
Attachment #8532806 - Flags: approval-comm-aurora?
Attachment #8532806 - Flags: approval-comm-aurora+
Posted patch Patch for ESRSplinter Review
Magnus, is this what you thought?
Attachment #8540625 - Flags: review?(mkmelin+mozilla)
Comment on attachment 8540625 [details] [diff] [review]
Patch for ESR

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

Yup, r=mkmelin
Attachment #8540625 - Flags: review?(mkmelin+mozilla) → review+
Comment on attachment 8540625 [details] [diff] [review]
Patch for ESR

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: user could be unable to add accounts
Testing completed (on c-c, etc.): full patch is on c-c
Risk to taking this patch (and alternatives if risky): low
Attachment #8540625 - Flags: approval-comm-esr31?
Attachment #8540625 - Flags: approval-comm-esr31? → approval-comm-esr31+
You need to log in before you can comment on or make changes to this bug.