getStringPref in clicktoplay-rollout addon returns null but was expected to return undefined

VERIFIED FIXED in Firefox 56

Status

()

defect
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: Felipe, Assigned: Felipe)

Tracking

56 Branch
Firefox 57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 unaffected, firefox56+ verified, firefox57+ verified)

Details

Attachments

(1 attachment)

+++ This bug was initially created as a clone of Bug #1397379 +++

In bug 1357517, Preferences.get was replaced by getStringPref.  In bug 1397379, we added an `undefined` parameter to it as the default return value, to avoid it throwing an error.

That worked in preventing the error from being thrown, but turns out that the value returned is actually `null` instead of `undefined` (as documented in bug 1357517 comment 63), but the switch statement below only checks for `undefined`.


[Tracking Requested - why for this release]:
Breaks the Flash Click-to-Play rollout for fresh profiles. Note: this is *not* a regression from bug 1390705. It is from bug 1357517, which should have been fixed by 1397379, but the fix was incomplete.
Summary: getStringPref in clicktoplay-rollout addon throws for new profiles → getStringPref in clicktoplay-rollout addon returns null but was expected to return undefined
Comment on attachment 8909869 [details]
Bug 1401249 - Fix getStringPref usage in clicktoplay-rollout.

https://reviewboard.mozilla.org/r/181366/#review186590
Attachment #8909869 - Flags: review?(gijskruitbosch+bugs) → review+
Pushed by felipc@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/d94508124715
Fix getStringPref usage in clicktoplay-rollout. r=Gijs
Comment on attachment 8909869 [details]
Bug 1401249 - Fix getStringPref usage in clicktoplay-rollout.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1357517
[User impact if declined]: Flash will fail to go Click-to-Play for *new* profiles.
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Not applicable, as it depends on the update channel being "release"
[Needs manual test from QE? If yes, steps to reproduce]: Yes. Mihai discovered this on bug 1390705. Once we have a new RC with this fix in place, he should retest it.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Just fixes the flash clicktoplay rollout, doesn't affect any other areas of the code.
[String changes made/needed]: none
Attachment #8909869 - Flags: approval-mozilla-release?
Comment on attachment 8909869 [details]
Bug 1401249 - Fix getStringPref usage in clicktoplay-rollout.

Fix for click to play system addon, so that new users will get this option. 
This should be in the 56 RC2 later on this week.
Attachment #8909869 - Flags: approval-mozilla-release? → approval-mozilla-release+
https://hg.mozilla.org/mozilla-central/rev/d94508124715
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
I managed to verify this issue on Firefox 57.0a1 (2017-09-19) (64-bit), under Windows 10x64.
Click-to-Play staged rollout is set to 1.4 by default and Shockwave Flash is set to 'Ask to Activate' by default. 
I'm not marking this issue Verified Fix until I can confirm the fix also on Firefox 56.
I managed to verify this issue also on Firefox 56.0-build3, under Windows 10x64, macOS 10.12 and under Ubuntu 16.04x64. Shockwave Flash is set by default to 'Ask to Activate' and Click-to-Play staged rollout is set to 1.4.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.