Closed Bug 855371 Opened 8 years ago Closed 8 years ago

@MOZ_UPDATE_CHANNEL@ in health report prefs

Categories

(Firefox Health Report Graveyard :: Client: Desktop, defect)

defect
Not set
normal

Tracking

(firefox22 verified, firefox23 verified)

VERIFIED FIXED
Firefox 23
Tracking Status
firefox22 --- verified
firefox23 --- verified

People

(Reporter: gps, Assigned: rnewman)

References

Details

Attachments

(2 files)

I opened about:config in today's Nightly and noticed that @MOZ_UPDATE_CHANNEL@ is in a healthreport pref:

datareporting.healthreport.service.providerCategories;healthreport-js-provider-default,healthreport-js-provider-@MOZ_UPDATE_CHANNEL@

That file (getting merged into greprefs.js) is supposed to be preprocessed. And, I thought this was verified to work in bug 853138. Not sure what's going on here. Gah.
Yeah, I verified this. humph.
The only thing I can think of is that if you switch a file from copy to preprocess, that effectively changes a symlink to a new file. When we write a file on top of an existing symlink, bad things can happen. Clobber fixes that. However, Nightly builds should be clobber, so...
Component: Metrics and Firefox Health Report → Client: Desktop
Product: Mozilla Services → Firefox Health Report
So I'm pretty sure I've done some clobber builds in the past week, and yet greprefs.js:

pref("datareporting.healthreport.service.providerCategories",
//@line 28 "/Users/rnewman/moz/hg/services-central/services/healthreport/healthreport-prefs.js"
    "healthreport-js-provider-default"
//@line 32 "/Users/rnewman/moz/hg/services-central/services/healthreport/healthreport-prefs.js"
    );

but in my Nightly:

pref("datareporting.healthreport.service.providerCategories",
//@line 30 "/builds/slave/m-cen-osx64-ntly-0000000000000/build/services/healthreport/healthreport-prefs.js"
    "healthreport-js-provider-default,healthreport-js-provider-@MOZ_UPDATE_CHANNEL@"
//@line 32 "/builds/slave/m-cen-osx64-ntly-0000000000000/build/services/healthreport/healthreport-prefs.js"
    );


That means that the file is being preprocessed, but it's not being substituted correctly.
Just did a clobber build and package. greprefs in my omni.ja is preprocessed correctly.
Though I should note that I'm using update-channel=release, so I'm not actually testing the substitution.
Guessed it might be a missing #filter substitution, and gavin confirmed.

The fun now is that custom builds look like this:

//@line 27 "/Users/rnewman/moz/hg/services-central/services/healthreport/healthreport-prefs.js"
pref("datareporting.healthreport.service.providerCategories",
//@line 31 "/Users/rnewman/moz/hg/services-central/services/healthreport/healthreport-prefs.js"
    "healthreport-js-provider-default,healthreport-js-provider-default"
//@line 33 "/Users/rnewman/moz/hg/services-central/services/healthreport/healthreport-prefs.js"
    );

so we might need to hack up that conditional some more.
Keep it simple.
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
Attachment #734071 - Flags: review?(gps)
Comment on attachment 734071 [details] [diff] [review]
Proposed patch. v1

I'd put the #filter line at the top of the file, since that's where people usually put it (and it applies to the whole file).
Comment on attachment 734071 [details] [diff] [review]
Proposed patch. v1

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

::: services/healthreport/healthreport-prefs.js
@@ +22,5 @@
>  pref("datareporting.healthreport.service.enabled", true);
>  pref("datareporting.healthreport.service.loadDelayMsec", 10000);
>  pref("datareporting.healthreport.service.loadDelayFirstRunMsec", 60000);
>  
> +#filter substitution

Put this at the top of the file since it applies to the entire file.
Attachment #734071 - Flags: review?(gps) → review+
https://hg.mozilla.org/mozilla-central/rev/225be6d5e41b
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in services]
[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
  853138

User impact if declined: 
  None visible, but prefs will contain a weird value.

Testing completed (on m-c, etc.): 
  Landed on m-c. No breakage on s-c, where it's been for a while.

Risk to taking this patch (and alternatives if risky): 
  Very low.
 
String or IDL/UUID changes made by this patch:
  None.
Attachment #737789 - Flags: review+
Attachment #737789 - Flags: approval-mozilla-aurora?
Target Milestone: --- → Firefox 23
Attachment #737789 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(In reply to Gregory Szorc [:gps] from comment #11)
> https://hg.mozilla.org/mozilla-central/rev/225be6d5e41b

Verified fixed in Firefox Nightly 23.0a1 2013-04-17.
Status: RESOLVED → VERIFIED
(In reply to Ryan VanderMeulen [:RyanVM] from comment #13)
> https://hg.mozilla.org/releases/mozilla-aurora/rev/cdec0ba6a574

Verified fixed in Firefox Aurora 22.0a2 2013-04-18.
Product: Firefox Health Report → Firefox Health Report Graveyard
You need to log in before you can comment on or make changes to this bug.