Closed Bug 627811 Opened 10 years ago Closed 10 years ago

Port bug 624970 (display DirectWrite version in about:support)

Categories

(Thunderbird :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.3a3

People

(Reporter: rain1, Assigned: rain1)

Details

Attachments

(1 file)

Attached patch patchSplinter Review
No description provided.
Attachment #505879 - Flags: review?(bwinton)
oops at the whitespace -- removed.
Comment on attachment 505879 [details] [diff] [review]
patch

>+++ b/mail/components/about-support/content/gfx.js
>@@ -92,17 +92,17 @@ function populateGraphicsSection() {
>         d2dStatus = gfxInfo.getFeatureStatus(gfxInfo.FEATURE_DIRECT2D);
>       } catch(e) {
>         window.dump(e + '\n');
>-      }
>+      }  

I’m not sure why you added a little trailing whitespace here, but we should probably not commit this part.  ;)

>@@ -117,22 +117,26 @@ function populateGraphicsSection() {
>     var dwEnabled = false;
>+    var dwriteEnabledStr = dwEnabled.toString();
>+    var dwriteVersion;
>     try {
>       dwEnabled = gfxInfo.DWriteEnabled;
>+      dwriteVersion = gfxInfo.DWriteVersion;
>+      dwriteEnabledStr = dwEnabled.toString() + " (" + dwriteVersion + ")";
>     } catch(e) {}
>     trGraphics.push(createParentElement("tr", [
>       createHeader(bundle.GetStringFromName("directWriteEnabled")),
>-      createElement("td", dwEnabled),
>+      createElement("td", dwriteEnabledStr),
>     ]));

You’re not really using dwEnabled or dwriteVersion outside of the try, so I suggest moving them inside the try, and only exposing dwriteEnabledStr.  (Okay, I know they’re all vars, and so they’re all exposed to the whole function, but you know what I mean.)

r=me with those changes.

Later,
Blake.
Attachment #505879 - Flags: review?(bwinton) → review+
(In reply to comment #2)
> 
> >@@ -117,22 +117,26 @@ function populateGraphicsSection() {
> >     var dwEnabled = false;
> >+    var dwriteEnabledStr = dwEnabled.toString();
> >+    var dwriteVersion;
> >     try {
> >       dwEnabled = gfxInfo.DWriteEnabled;
> >+      dwriteVersion = gfxInfo.DWriteVersion;
> >+      dwriteEnabledStr = dwEnabled.toString() + " (" + dwriteVersion + ")";
> >     } catch(e) {}
> >     trGraphics.push(createParentElement("tr", [
> >       createHeader(bundle.GetStringFromName("directWriteEnabled")),
> >-      createElement("td", dwEnabled),
> >+      createElement("td", dwriteEnabledStr),
> >     ]));
> 
> You’re not really using dwEnabled or dwriteVersion outside of the try, so I
> suggest moving them inside the try, and only exposing dwriteEnabledStr.  (Okay,
> I know they’re all vars, and so they’re all exposed to the whole function, but
> you know what I mean.)

So, well, I'd like to keep this consistent with core, especially since it's such a minor change.
Status: NEW → ASSIGNED
https://hg.mozilla.org/comm-central/rev/b46dadaa931e
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.3a3
Version: unspecified → Trunk
You need to log in before you can comment on or make changes to this bug.