Closed Bug 1349689 Opened 7 years ago Closed 7 years ago

Remove old preferences fork (in-content-old)

Categories

(Firefox :: Settings UI, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 57
Tracking Status
firefox57 + fixed

People

(Reporter: jaws, Assigned: rickychien)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [photon-preference])

Attachments

(1 file, 1 obsolete file)

The old preferences fork should be removed once we are satisfied with the state of the preferences reorg. This should remove the /browser/components/preferences/in-content-old/ directory, and also undo the changes in allowed-dupes.mn.
[Tracking Requested - why for this release]: For the sake of our localizers we should stop maintaining and shipping the old preferences implementation once we are satisfied with the release of the new preferences in Firefox 56.
Tracked for 57. Hi Jared, we are in the Nightly 57 cycle, is this a few hours of dev time and achievable well before 9/21 Merge day? Or is this better off tracked for 58+?
Flags: needinfo?(jaws)
This is only a few hours of dev time and something that we should do within the 57 Nightly cycle. The plan here is to wait a week or two of 56 Beta to make sure there are no large issues found with the new organization that would prevent us from shipping and require us to still use the old preferences.
Flags: needinfo?(jaws)
Whiteboard: [photon-preference][triage]
Flags: qe-verify-
Priority: -- → P2
Whiteboard: [photon-preference][triage] → [photon-preference]
This task will include

* Remove browser/components/preferences/in-content folder.
* Remove browser/themes/shared/incontentprefs-old folder.
* Remove browser.preferences.useOldOrganization flag and related if else condition in codebase.
* Anything else related to old preferences folk
Depends on: 1391305
Assignee: nobody → rchien
Status: NEW → ASSIGNED
Priority: P2 → P1
Target Milestone: --- → Firefox 57
Version: 52 Branch → unspecified
(In reply to Ricky Chien [:rickychien] from comment #4)
> * Remove browser/components/preferences/in-content folder.
> * Remove browser/themes/shared/incontentprefs-old folder.

That should also include strings used only in the in-content preferences. 

Does it make sense to keep around the strings in preferences-old for a bit more (filing a follow-up bug for removal), in case we might need them back? Is there any risk that this change (switch to reorganize prefs) will be questioned in the future?
The goal of this bug is aiming to removing old preferences once we reach consensus and there is no big issue from beta users after a few weeks. And then after removal, I believe we don't have any chance to get back to the old version.

I'd prefer to remove all legacy code in one sweep. But I'd like to know is there any concerned from localization process that it would be better if we file a follow-up bug for l10n removal?
Flags: needinfo?(francesco.lodolo)
(In reply to Ricky Chien [:rickychien] from comment #9)
> I'd prefer to remove all legacy code in one sweep. But I'd like to know is
> there any concerned from localization process that it would be better if we
> file a follow-up bug for l10n removal?

My concern is only one: if we remove the old preferences, and someone decides that we need to go back, it's really easy to backout the code removal, it's basically impossible to recover those strings from a hundred l10n repositories in different states.

As long as code is removed after there's no going back, we can do l10n and code together.
Flags: needinfo?(francesco.lodolo)
It makes sense to me. I'll file a separate bug to deal with l10n removal. Thanks
(In reply to Francesco Lodolo [:flod] from comment #10)
> (In reply to Ricky Chien [:rickychien] from comment #9)
> > I'd prefer to remove all legacy code in one sweep. But I'd like to know is
> > there any concerned from localization process that it would be better if we
> > file a follow-up bug for l10n removal?
> 
> My concern is only one: if we remove the old preferences, and someone
> decides that we need to go back, it's really easy to backout the code
> removal, it's basically impossible to recover those strings from a hundred
> l10n repositories in different states.
> 
> As long as code is removed after there's no going back, we can do l10n and
> code together.

Unfortunately, I received lots of unreferenced file failures like https://treeherder.mozilla.org/#/jobs?repo=try&revision=1f695f98e2e5088c8dff53042bcac99e7b9c59a1&selectedJob=125802863. I think it's still make sense to remove unused l10n file while removing old preferences folder.
(In reply to Ricky Chien [:rickychien] from comment #17)
> Unfortunately, I received lots of unreferenced file failures like
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=1f695f98e2e5088c8dff53042bcac99e7b9c59a1&selectedJob=1
> 25802863. I think it's still make sense to remove unused l10n file while
> removing old preferences folder.

OK. In the meantime, we moved all our localizations to Pontoon, which doesn't remove obsolete files by default, so the impact of of a backout could be less painful (only a handful of locales).
Attachment #8899674 - Attachment is obsolete: true
Attachment #8899674 - Flags: review?(jaws)
Comment on attachment 8902557 [details]
Bug 1349689 - Remove old preferences fork

https://reviewboard.mozilla.org/r/174166/#review179560

rs=me. I built it and tested it out, as well as searched the codebase for "in-content-new" and "in-content-old" and found no matches.
Attachment #8902557 - Flags: review?(jaws) → review+
Blocks: 1395426
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 8cc07a51bb9c -d 08122b5e49f8: rebasing 417001:8cc07a51bb9c "Bug 1349689 - Remove old preferences fork r=jaws" (tip)
local [dest] changed browser/components/preferences/in-content-new/main.js which other [source] deleted
use (c)hanged version, (d)elete, or leave (u)nresolved? u
local [dest] changed browser/components/preferences/in-content-new/main.xul which other [source] deleted
use (c)hanged version, (d)elete, or leave (u)nresolved? u
local [dest] changed browser/components/preferences/in-content-new/preferences.js which other [source] deleted
use (c)hanged version, (d)elete, or leave (u)nresolved? u
local [dest] changed browser/components/preferences/in-content-new/privacy.js which other [source] deleted
use (c)hanged version, (d)elete, or leave (u)nresolved? u
local [dest] changed browser/components/preferences/in-content-new/tests/browser_bug1020245_openPreferences_to_paneContent.js which other [source] deleted
use (c)hanged version, (d)elete, or leave (u)nresolved? u
local [dest] changed browser/components/preferences/in-content-new/tests/browser_search_within_preferences_1.js which other [source] deleted
use (c)hanged version, (d)elete, or leave (u)nresolved? u
local [dest] changed browser/components/preferences/in-content/security.js which other [source] deleted
use (c)hanged version, (d)elete, or leave (u)nresolved? u
local [dest] changed browser/components/preferences/in-content/tests/browser_security.js which other [source] deleted
use (c)hanged version, (d)elete, or leave (u)nresolved? u
merging browser/app/profile/firefox.js
merging browser/base/content/browser.js
merging browser/base/content/utilityOverlay.js
merging browser/components/preferences/in-content/containers.xul
merging browser/components/preferences/in-content-new/tests/browser_search_subdialogs_within_preferences_1.js and browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_1.js to browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_1.js
merging browser/components/preferences/in-content-new/tests/browser_security-2.js and browser/components/preferences/in-content/tests/browser_security-2.js to browser/components/preferences/in-content/tests/browser_security-2.js
merging browser/themes/shared/incontentprefs/preferences.inc.css
merging testing/runtimes/mochitest-browser-chrome-e10s.runtimes.json
merging testing/runtimes/mochitest-browser-chrome.runtimes.json
merging toolkit/content/aboutTelemetry.js
warning: conflicts while merging testing/runtimes/mochitest-browser-chrome-e10s.runtimes.json! (edit, then use 'hg resolve --mark')
warning: conflicts while merging testing/runtimes/mochitest-browser-chrome.runtimes.json! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 8850a2bd06b5 -d 93ed0305b3a3: rebasing 417082:8850a2bd06b5 "Bug 1349689 - Remove old preferences fork r=jaws" (tip)
local [dest] changed browser/components/preferences/in-content-new/preferences.js which other [source] deleted
use (c)hanged version, (d)elete, or leave (u)nresolved? u
local [dest] changed browser/components/preferences/in-content-new/tests/browser_bug1020245_openPreferences_to_paneContent.js which other [source] deleted
use (c)hanged version, (d)elete, or leave (u)nresolved? u
merging browser/app/profile/firefox.js
merging browser/components/nsBrowserGlue.js
merging browser/components/preferences/in-content/containers.xul
merging browser/extensions/formautofill/FormAutofillParent.jsm
merging toolkit/content/aboutTelemetry.js
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by rchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/df8215261c7e
Remove old preferences fork r=jaws
Pushed by rchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9db0cec976bb
Remove old preferences fork r=jaws
This reduced the installer size on Windows 64 PGO by 100 KB, thanks!
Depends on: 1400829
Blocks: 1405237
Blocks: 1340993
You need to log in before you can comment on or make changes to this bug.