Make the tabGroups.enabled Nimbus feature variable use setPref instead of fallbackPref
Categories
(Firefox :: Tabbed Browser, defect, P1)
Tracking
()
People
(Reporter: dao, Assigned: dao)
References
(Blocks 1 open bug)
Details
(Whiteboard: [fidefe-tabgrps])
Attachments
(2 files)
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
phab-bot
:
approval-mozilla-beta+
|
Details | Review |
Per Beth, setPref is preferable over fallbackPref:
This tabGroups.enabled feature variable is using fallbackPref. That means that when there is no experiment value for the variable, we fallback to reading the pref specified in the manifest. However, it also means that if there is an experiment value for the variable, changing the underlying pref will have no effect.
This is especially important in the case where a user gets put into the control branch with enabled=false because if they discover the feature independently and decide to toggle it on via the pref it will not get enabled and they might get confused (edited)
Additionally, this recipe can target people that have already toggled this pref to true and its possible they get put in the control branch with enabled=false. That means they will lose access to the feature (and will definitely be confused)
At a minimum we should add an advanced targeting to prevent that case. We cannot prevent people being enrolled into the control and then turning on the feature, but we can prevent degrading the experience of users that have already opted into the feature
(sidenote: fallbackPref is deprecated and is slated for removal this year. We have a different feature called setPref which allows Nimbus to write to prefs directly. Going forward we are recommending everyone switch over to setPref because it avoids all these issues)
We'll try to fix this in 137.
Updated•1 year ago
|
| Assignee | ||
Comment 1•1 year ago
|
||
Updated•1 year ago
|
| Assignee | ||
Comment 2•1 year ago
|
||
Updated•1 year ago
|
Comment 3•1 year ago
|
||
beta Uplift Approval Request
- User impact if declined: See https://bugzilla.mozilla.org/show_bug.cgi?id=1955621#c0
- Code covered by automated testing: no
- Fix verified in Nightly: no
- Needs manual QE test: no
- Steps to reproduce for manual QE testing: -
- Risk associated with taking this patch: Low
- Explanation of risk level: Reviewed this with Beth from the Nimbus team, and tested locally that the feature still works via nimbus devtools
- String changes made/needed: -
- Is Android affected?: no
Comment 5•1 year ago
|
||
| bugherder | ||
Updated•1 year ago
|
Updated•1 year ago
|
Description
•