940806 - Gfx info is not being properly reported in Telemetry

Status: RESOLVED FIXED

Description

[Vladan Djeric (:vladan)]

Created attachment 8335009 [details] [diff] [review]
Do a strict inequality comparison

We're not reporting any of the gfx system info fields whose value is boolean "false" because of a bad comparison in TelemetryPing.js

> false != ""
false

> false !== ""
true

Comment 1

[David Teller [:Yoric] (please use "needinfo")]

Comment on attachment 8335009 [details] [diff] [review]
Do a strict inequality comparison

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

::: toolkit/components/telemetry/TelemetryPing.js
@@ +373,5 @@
>      if (gfxInfo) {
>        for each (let field in gfxfields) {
>          try {
>            let value = "";
>            value = gfxInfo[field];

Could you take the opportunity to make this
 let value = gfxInfo(field);
?

@@ +374,5 @@
>        for each (let field in gfxfields) {
>          try {
>            let value = "";
>            value = gfxInfo[field];
> +          if (value !== "") {

You'll need a comment to explain why you need a strict comparison here (i.e. it's not evident that we're comparing booleans with strings in some cases or that |"" == false|).

Comment 2

[Vladan Djeric (:vladan)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/038a45f0c9d5

Comment 3

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/038a45f0c9d5