Closed
Bug 1343682
Opened 8 years ago
Closed 8 years ago
Create temporary killswitch for preference reorg
Categories
(Firefox :: Settings UI, enhancement)
Firefox
Settings UI
Tracking
()
RESOLVED
FIXED
Firefox 55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: mconley, Assigned: jaws)
References
Details
Attachments
(1 file)
Bug 1335907 has us reorganizing about:preferences. We think we want to ship this with the (_temporary_) ability to shut it off if it turns out that there are accessibility, localization, add-on, or other correctness problems with it. This is not a preference that is going to stick around - we're hoping for maximum of 5 releases. If it turns out that we just can't get the pref re-org shipped without the killswitch in that time, we'll just back the whole thing out.
Comment hidden (mozreview-request) |
Reporter | ||
Comment 2•8 years ago
|
||
mozreview-review |
Comment on attachment 8845117 [details] Bug 1343682 - Create temporary killswitch for preference reorg. https://reviewboard.mozilla.org/r/118334/#review120184 ::: browser/components/about/AboutRedirector.cpp:154 (Diff revision 1) > do_GetService("@mozilla.org/browser/aboutnewtab-service;1", &rv); > NS_ENSURE_SUCCESS(rv, rv); > rv = aboutNewTabService->GetDefaultURL(url); > NS_ENSURE_SUCCESS(rv, rv); > + } else if (path.EqualsLiteral("preferences") && > + Preferences::GetBool("browser.preferences.useOldOrganization", false)) { I seem to recall this [dev-platform post](https://groups.google.com/d/msg/mozilla.dev.platform/ZmHWTH_trrA/T0PW1gcZCQAJ) about trying to avoid Preferences::GetBool, and to use Preferences::AddBoolVarCache instead. Do you think it's worth doing here? ::: browser/components/preferences/moz.build:7 (Diff revision 1) > # vim: set filetype=python: > # This Source Code Form is subject to the terms of the Mozilla Public > # License, v. 2.0. If a copy of the MPL was not distributed with this > # file, You can obtain one at http://mozilla.org/MPL/2.0/. > > -DIRS += ['in-content'] > +DIRS += ['in-content-old', 'in-content'] Let's break this over a few lines, like with BROWSER_CHROME_MANIFESTS
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Reporter | ||
Comment 3•8 years ago
|
||
mozreview-review |
Comment on attachment 8845117 [details] Bug 1343682 - Create temporary killswitch for preference reorg. https://reviewboard.mozilla.org/r/118334/#review120604 Clearing review flag until the next patch.
Attachment #8845117 -
Flags: review?(mconley)
Comment hidden (mozreview-request) |
Reporter | ||
Comment 5•8 years ago
|
||
mozreview-review |
Comment on attachment 8845117 [details] Bug 1343682 - Create temporary killswitch for preference reorg. https://reviewboard.mozilla.org/r/118334/#review121094 I suspect we'll want to modify in-content-old/tests/head.js to flip the pref, right? Otherwise looks good. Thanks!
Attachment #8845117 -
Flags: review?(mconley) → review+
Assignee | ||
Comment 6•8 years ago
|
||
(In reply to Mike Conley (:mconley) (Catching up on reviews and needinfos) from comment #5) > I suspect we'll want to modify in-content-old/tests/head.js to flip the > pref, right? Yes, OMG good catch! :P
Comment hidden (mozreview-request) |
Pushed by jwein@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c00c5431182b Create temporary killswitch for preference reorg. r=mconley
This had build failures like this show up https://treeherder.mozilla.org/logviewer.html#?job_id=83115538&repo=autoland Backed out in https://hg.mozilla.org/integration/autoland/rev/4845ecb9f6c33410a39fc6faf81f0783f6bf5bd6
Flags: needinfo?(jaws)
Assignee | ||
Comment 10•8 years ago
|
||
Okay, this is failing because we are landing duplicate files. We can wait until the reorg patch is ready to land before we land this, and then land them in the same push.
Flags: needinfo?(jaws)
Assignee | ||
Comment 11•8 years ago
|
||
For posterity, https://hg.mozilla.org/mozilla-central/rev/532415dbd620 is what does the duplicate checking. Once we land the reorg we will still have some duplicate files (containers.js for example), so we'll need to update this list. The duplicate list doesn't look like it will throw an error if a file is listed that is not actually a dupe, so we may just want to add all of the 'in-content-old' directory to the list since the directory is intentionally a dupe and at some point a file may inadvertently match the 'in-content' directory and this test failure would be unexpected though allowable.
Reporter | ||
Comment 12•8 years ago
|
||
Hey - are we still good to re-land this with the linter ignoring the duplication?
Flags: needinfo?(jaws)
Assignee | ||
Comment 13•8 years ago
|
||
(In reply to Mike Conley (:mconley) (Catching up on reviews and needinfos) from comment #12) > Hey - are we still good to re-land this with the linter ignoring the > duplication? Yeah we are. I'd like to coordinate them closer together though. The steps to create this patch are pretty simple, and I will re-create it when we are ready. 1. hg cp browser/components/preferences/in-content browser/components/preferences/in-content-old 2. s/in-content/in-content-old/ within "in-content-old" 3. Apply the same changes to AboutRedirector.{h,cpp} and browser/components/preferences/moz.build 4. Add the full list of "in-content-old" to find-dupes.py
Flags: needinfo?(jaws)
Assignee | ||
Comment 14•8 years ago
|
||
Note also we will need to fork the strings. Working on this now...
Comment hidden (mozreview-request) |
Comment 16•8 years ago
|
||
Pushed by jwein@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f23ce5b464f8 Create temporary killswitch for preference reorg. r=mconley
I had to back this out for build bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=85741129&repo=autoland https://hg.mozilla.org/integration/autoland/rev/9fed934aa4af5a894b9b053fabff2886c91a9d7c
Flags: needinfo?(jaws)
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(jaws)
Comment hidden (mozreview-request) |
Comment 19•8 years ago
|
||
Pushed by jwein@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0847b9063bf7 Create temporary killswitch for preference reorg. r=mconley
Backed out again for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=85765829&repo=autoland https://hg.mozilla.org/integration/autoland/rev/8f6a5253b032
Flags: needinfo?(jaws)
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(jaws)
Comment hidden (mozreview-request) |
Comment 22•8 years ago
|
||
Pushed by jwein@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/892ffc32ee08 Create temporary killswitch for preference reorg. r=mconley
Comment 23•8 years ago
|
||
Pushed by philringnalda@gmail.com: https://hg.mozilla.org/integration/autoland/rev/21f183c27eba followup, touch CLOBBER since the previous backout didn't actually stop the failures
Comment 24•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/892ffc32ee08 https://hg.mozilla.org/mozilla-central/rev/21f183c27eba
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
You need to log in
before you can comment on or make changes to this bug.
Description
•