Closed Bug 1675551 Opened 6 months ago Closed 6 months ago

Enable use of 2h2020 bookmarks pref for experiments

Categories

(Firefox :: Bookmarks & History, defect, P3)

Desktop
All
defect

Tracking

()

RESOLVED FIXED
84 Branch
Tracking Status
firefox84 --- fixed

People

(Reporter: Gijs, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

To be able to reliably flip the pref for a subset of our userbase, we need to make sure that:

  1. we ship with the pref enabled to early beta so we find issues on beta
  2. all migrations in BrowserGlue.jsm can deal with the pref being either on or off, and we reliably migrate prefs / settings when the pref is flipped, irrespective of what version that happens in.
  3. audit use of the pref / other deps of bug 1665353 to make sure all the code that needs to check it does check it.

I think the easiest is to do this work in a single bug as it is likely interdependent. We already know of one thing where we cannot use the pref reliably, which is the shortcut for the toolbar which is covered in bug 1675549.

Severity: -- → S3
Priority: -- → P3

(In reply to :Gijs (he/him) from comment #0)

  1. all migrations in BrowserGlue.jsm can deal with the pref being either on or off, and we reliably migrate prefs / settings when the pref is flipped, irrespective of what version that happens in.

So the migrations look like this:

    // We have to rerun these because we had to use 102 on beta.
    // They were 101 and 102 before.
    if (currentUIVersion < 103) {
      // Set a pref if the bookmarks toolbar was already visible,
      // so we can keep it visible when navigating away from newtab
      let bookmarksToolbarWasVisible =
        Services.xulStore.getValue(
          BROWSER_DOCURL,
          "PersonalToolbar",
          "collapsed"
        ) == "false";
      if (bookmarksToolbarWasVisible) {
        // Migrate the user to the "always visible" value. See firefox.js for
        // the other possible states.
        Services.prefs.setCharPref(
          "browser.toolbars.bookmarks.visibility",
          "always"
        );
      }
      Services.xulStore.removeValue(
        BROWSER_DOCURL,
        "PersonalToolbar",
        "collapsed"
      );

      Services.prefs.clearUserPref(
        "browser.livebookmarks.migrationAttemptsLeft"
      );
    }

    // For existing profiles, continue putting bookmarks in the
    // "other bookmarks" folder.
    if (currentUIVersion < 104) {
      Services.prefs.setCharPref(
        "browser.bookmarks.defaultLocation",
        "unfiled"
      );
    }

For the bookmarks toolbar, AIUI we're shifting away from the use of persistence to govern whether the toolbar is collapsed or not, and we're doing that irrespective of the 2020h2 pref, so that's OK.

Similarly, we ignore the defaultLocation pref iff the 2020h2 pref is false. Though I guess we should also avoid writing to it in that case...

Jared, can you confirm that's right?

Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Flags: needinfo?(jaws)

Yeah that sounds right to me.

Flags: needinfo?(jaws)
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/a6b0c42f1e6a
ensure we can use the browser.toolbars.bookmarks.2h2020 pref for experiments and enable it on early beta, r=jaws
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → 84 Branch
You need to log in before you can comment on or make changes to this bug.