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

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Rain [:rain1] (inactive)]

Attachment: patch

Comment 1

[Rain [:rain1] (inactive)]

oops at the whitespace -- removed.

Comment 2

[Blake Winton (:bwinton) (:☕️)]

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.

Comment 3

[Rain [:rain1] (inactive)]

(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.

Comment 4

[Rain [:rain1] (inactive)]

https://hg.mozilla.org/comm-central/rev/b46dadaa931e