Remove old preferences fork (in-content-old)

RESOLVED FIXED in Firefox 57

Status

()

enhancement
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jaws, Assigned: rickychien)

Tracking

(Blocks 2 bugs)

unspecified
Firefox 57
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox57+ fixed)

Details

(Whiteboard: [photon-preference])

Attachments

(1 attachment, 1 obsolete attachment)

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

Updated

2 years ago
Depends on: 1391305
Duplicate of this bug: 1386051
Duplicate of this bug: 1391305
Assignee: nobody → rchien
Status: NEW → ASSIGNED
Priority: P2 → P1
Target Milestone: --- → Firefox 57
Version: 52 Branch → unspecified
Comment hidden (mozreview-request)
(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
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(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).
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
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
Comment hidden (mozreview-request)

Comment 37

2 years ago
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)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 41

2 years ago
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)
Comment hidden (mozreview-request)

Comment 43

2 years ago
Pushed by rchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/df8215261c7e
Remove old preferences fork r=jaws
Comment hidden (mozreview-request)

Comment 46

2 years ago
Pushed by rchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9db0cec976bb
Remove old preferences fork r=jaws
https://hg.mozilla.org/mozilla-central/rev/9db0cec976bb
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
This reduced the installer size on Windows 64 PGO by 100 KB, thanks!
Depends on: 1400829
Blocks: 1405237
You need to log in before you can comment on or make changes to this bug.