Closed
Bug 1395419
Opened 7 years ago
Closed 7 years ago
Migrate values of urlclassifier.malwareTable that were customized via about:preferences
Categories
(Toolkit :: Safe Browsing, enhancement, P1)
Toolkit
Safe Browsing
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox56 | --- | unaffected |
firefox57 | --- | fixed |
People
(Reporter: francois, Assigned: tnguyen)
Details
(Whiteboard: #sbv4-m9)
Attachments
(1 file, 1 obsolete file)
If a user unticked "Warn you about unwanted and uncommon software" prior to upgrading to Firefox 56, then they have a non-default value of urlclassifier.malwareTable: goog-malware-shavar,test-malware-simple During the upgrade to 56, we should replace that with the following: goog-malware-proto,test-malware-simple
Reporter | ||
Comment 1•7 years ago
|
||
We could also attempt to fix the prefs that were customized via about:config by going through urlclassifier.*Table and then replacing the known goog-*-shavar lists with the V4 equivalents. However, that's lower priority (and should be done in a different bug) because we don't support users editing random prefs via about:config. That's something we can think about in 58.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → tnguyen
Assignee | ||
Comment 2•7 years ago
|
||
MozReview-Commit-ID: FSWBOv4wIgT
Assignee | ||
Comment 3•7 years ago
|
||
Comment on attachment 8904161 [details] [diff] [review] Migrate values of urlclassifier.malwareTable that were customized via about:preferences I think we only don't need to fix in nightly but only need to uplift the patch to beta (upgrade from 55-56).
Attachment #8904161 -
Flags: review?(francois)
Assignee | ||
Updated•7 years ago
|
Status: NEW → ASSIGNED
Reporter | ||
Comment 4•7 years ago
|
||
(In reply to Thomas Nguyen[:tnguyen] ni plz from comment #3) > I think we only don't need to fix in nightly but only need to uplift the > patch to beta (upgrade from 55-56). I think it needs to be fixed on mozilla-central, and then uplifted to Beta. I don't think we can have a higher `UI_VERSION` on Beta than on mozilla-central.
Reporter | ||
Comment 5•7 years ago
|
||
[Tracking Requested - why for this release]: Users who have customized the Safe Browsing preferences in about:preferences will not be getting a clean Safe Browsing V4 if we don't migrate their pref. They will be getting a mixture of V2 and V4.
tracking-firefox56:
--- → ?
Reporter | ||
Comment 6•7 years ago
|
||
Comment on attachment 8904161 [details] [diff] [review] Migrate values of urlclassifier.malwareTable that were customized via about:preferences Review of attachment 8904161 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/nsBrowserGlue.js @@ +1734,5 @@ > }, > > // eslint-disable-next-line complexity > _migrateUI: function BG__migrateUI() { > + const UI_VERSION = 51; Probably needs to be based on the version that's on mozilla-central right now otherwise Beta and central will be out of sync. @@ +2095,5 @@ > } > } > > + if (currentUIVersion < 51) { > + const MALWARE_PREF = "urlclassifier.malwareTable"; Add a comment like: // Update user customizations that will interfere with the Safe Browsing V2 to V4 migration (bug 1395419). @@ +2097,5 @@ > > + if (currentUIVersion < 51) { > + const MALWARE_PREF = "urlclassifier.malwareTable"; > + if (Services.prefs.prefHasUserValue(MALWARE_PREF)) { > + let malwareList = Services.prefs.getCharPref(MALWARE_PREF).split(","); I don't think you need to split/join the string. You could simply do: s = getCharPref() s.replace('goog-malware-shavar', 'goog-malware-proto') setCharPref(s)
Attachment #8904161 -
Flags: review?(francois) → review-
Reporter | ||
Comment 7•7 years ago
|
||
(In reply to François Marier [:francois] from comment #4) > (In reply to Thomas Nguyen[:tnguyen] ni plz from comment #3) > > I think we only don't need to fix in nightly but only need to uplift the > > patch to beta (upgrade from 55-56). > > I think it needs to be fixed on mozilla-central, and then uplifted to Beta. > > I don't think we can have a higher `UI_VERSION` on Beta than on > mozilla-central. Actually, thinking about it some more, it's more complicated than this. We'll be initially shipping V2 to 56 and then gradually rolling out V4. This means that we can't migrate the pref in 56. Instead, we'll have to: 1. migrate the pref in 57 via the mechanism your patch uses 2. migrate the pref on release 56 when we go to 100% (hotfix / point release)
tracking-firefox56:
? → ---
Assignee | ||
Updated•7 years ago
|
Attachment #8904161 -
Attachment is obsolete: true
Comment hidden (mozreview-request) |
Assignee | ||
Comment 9•7 years ago
|
||
Agree, I push a new patch with the same idea, but only for nightly. We may fix in nightly but leave this bug open until we could do the same with release 56. (Fennec need to be done in the same way but in nightly 58)
Reporter | ||
Comment 10•7 years ago
|
||
(In reply to Thomas Nguyen[:tnguyen] ni plz from comment #9) > (Fennec need to be done in the same way but in nightly 58) We don't need to do any migrations for Fennec since it doesn't have any UI prefs for Safe Browsing.
Reporter | ||
Comment 11•7 years ago
|
||
mozreview-review |
Comment on attachment 8904820 [details] Bug 1395419 - Migrate values of urlclassifier.malwareTable that were customized via about:preferences https://reviewboard.mozilla.org/r/176578/#review182074
Attachment #8904820 -
Flags: review?(francois) → review+
Reporter | ||
Updated•7 years ago
|
status-firefox56:
--- → unaffected
status-firefox57:
--- → affected
Comment 12•7 years ago
|
||
Pushed by tnguyen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/30832c0f384d Migrate values of urlclassifier.malwareTable that were customized via about:preferences r=francois
Comment 13•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/30832c0f384d
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•