Closed Bug 1328913 Opened 7 years ago Closed 7 years ago

Pref-off Search Reset UI for beta and release

Categories

(Firefox :: Search, defect, P1)

51 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 53
Tracking Status
firefox51 + verified
firefox52 --- verified
firefox53 --- verified

People

(Reporter: florian, Assigned: florian)

References

Details

(Whiteboard: [fxsearch])

Attachments

(2 files)

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

I'm not sure which version of Firefox we are targeting to run our Shield study on search reset, but in bug 1301784 we landed a patch to pref off the feature in beta 1 for the 50 cycle, and for the 51 cycle it seems we all forgot about it... and it's currently enabled on the beta channel.
Attached patch PatchSplinter Review
Let's do something with an ifdef this time, so that we can't forget it for the next cycle.
Attachment #8824100 - Flags: review?(past)
Attached patch Patch for beta51Splinter Review
RELEASE_BUILD was renamed to RELEASE_OR_BETA in the 52 cycle (bug 1304829) so beta51 needs a separate patch.
Attachment #8824101 - Flags: review?(past)
[Tracking Requested - why for this release]: because we don't want to ship Search Reset unintentionally in 51.
Attachment #8824100 - Flags: review?(past) → review+
Attachment #8824101 - Flags: review?(past) → review+
I see another instance of RELEASE_BUILD in the tree:

https://dxr.mozilla.org/mozilla-central/source/modules/libpref/init/all.js#1300

Do you know if it was intentionally left?
(In reply to Panos Astithas [:past] from comment #4)
> I see another instance of RELEASE_BUILD in the tree:
> 
> https://dxr.mozilla.org/mozilla-central/source/modules/libpref/init/all.
> js#1300
> 
> Do you know if it was intentionally left?

Seems very unlikely, I would guess bug 1309861 got bitrotted by bug 1304829.
Comment on attachment 8824100 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/Bug causing the regression]: The feature is bug 1203168, we don't want to ship it before doing a Shield study, which is currently being worked on.
[User impact if declined]: release users would get the search reset prompt before we know for sure which impact it has.
[Is this code covered by automated tests?]: Search Reset is well covered by tests, but this patch is just about preff'ing it off.
[Has the fix been verified in Nightly?]: Not yet.
[Needs manual test from QE? If yes, steps to reproduce]: Just verify in about:config that browser.search.reset.enabled is true on nightly and aurora, and false on beta/release.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Just preff'ing it off.
[String changes made/needed]: None
Attachment #8824100 - Flags: approval-mozilla-aurora?
Comment on attachment 8824101 [details] [diff] [review]
Patch for beta51

Approval Request Comment
See comment 7.
Attachment #8824101 - Flags: approval-mozilla-beta?
https://hg.mozilla.org/mozilla-central/rev/2cc7fd77a617
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Comment on attachment 8824100 [details] [diff] [review]
Patch

Add the logic to pref this off for 52.
Attachment #8824100 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8824101 [details] [diff] [review]
Patch for beta51

Pref off the search UI work for 51, for now. 
This should make it into beta 13.
Attachment #8824101 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flagging this for manual testing on 51b13 -- instructions available in Comment 7.
Flags: qe-verify+
Verified under Mac OS X 10.11.5 and Win 10 64-bit that the preference browser.search.reset.enabled is set to True on 53.0a1 and 52.0a2 2017-01-09 and to False on Firefox 51 beta 13.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: