The default bug view has changed. See this FAQ.

"Migration to a clean Firefox profile" feature doesn't suggest browser history migration

RESOLVED FIXED in Firefox 14

Status

()

Firefox
Migration
--
major
RESOLVED FIXED
5 years ago
a year ago

People

(Reporter: Alexander L. Slovesnik, Assigned: MattN)

Tracking

({regression})

Trunk
Firefox 15
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(firefox14+ fixed, firefox15 verified, firefox16 fixed)

Details

Attachments

(6 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Created attachment 619277 [details]
Screenshot in nightly

STR:
1) Open about:support
2) Click on "Reset Nightly"

Expected result:
"Browsing history" is present in list of migrated data

Expected result:
"Browsing history" is absent in list of migrated data

I use Mozilla/5.0 (X11; Linux i686; rv:15.0) Gecko/20120427 Firefox/15.0a1
(Reporter)

Comment 1

5 years ago
Created attachment 619278 [details]
Screenshot in 13.0b1

Screenshot in 13.0b1 for comparison
(Reporter)

Comment 2

5 years ago
Last good build - http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012-04-14-03-07-31-mozilla-central/firefox-14.0a1.en-US.win32.zip
First bad build - http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2012-04-15-03-07-25-mozilla-central/firefox-14.0a1.en-US.win32.zip

Regression window - http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=364f0a5a1d2d&tochange=0d871550085e

Perhaps caused by Bug 737381?
Keywords: regression
(In reply to Alexander L. Slovesnik from comment #2)
> Perhaps caused by Bug 737381?

Yes, it was caused by that bug.  The dialog UI code should now use MU_getLocalizedString to get the names of the items.
Assignee: nobody → mnoorenberghe+bmo
Status: NEW → ASSIGNED
status-firefox14: --- → affected
tracking-firefox14: --- → ?
Depends on: 737381
tracking-firefox14: ? → +
Created attachment 619675 [details] [diff] [review]
v.1 Use new MigrationUtils.getLocalizedString to handle string overrides

Thanks for the report Alexander! Note that this feature is broken on Nightly until bug 748047 is fixed.
Attachment #619675 - Flags: review?(mano)
Comment on attachment 619675 [details] [diff] [review]
v.1 Use new MigrationUtils.getLocalizedString to handle string overrides

MigrationUtils is a browser/ module.

So, I think you want to move these files to browser/, but keep them in the current chrome address. That way, each app would have to provide this ui, in that chrome address. That's better than forcing the app to provide locale files and modules...

Alternatively, about:support could be moved to browser/. I think the benefit of having it in toolkit is very limited.
Attachment #619675 - Flags: review?(mano) → review-
Created attachment 620856 [details] [diff] [review]
v.2 Use new MigrationUtils.getLocalizedString to handle string overrides (Cu.import in getMigratedData)

(In reply to Mano from comment #5)
> Comment on attachment 619675 [details] [diff] [review]
> v.1 Use new MigrationUtils.getLocalizedString to handle string overrides
> 
> MigrationUtils is a browser/ module.
> 
> So, I think you want to move these files to browser/, but keep them in the
> current chrome address. That way, each app would have to provide this ui, in
> that chrome address. That's better than forcing the app to provide locale
> files and modules...

It doesn't seem necessary to require an application to re-implement the dialog when they already have to implement the migrator.  I moved the import into getMigratedData to address the issue since resetSupported already checks that a migrator for the app exists since migrators are also browser components.  Also keep in mind that this change needs to land on Aurora.

> Alternatively, about:support could be moved to browser/. I think the benefit
> of having it in toolkit is very limited.

That could be moved as a separate bug if desired but it shouldn't block this follow-up fix caused by a string change.  Note that other applications are using it.
Attachment #619675 - Attachment is obsolete: true
Attachment #620856 - Flags: review?(mano)
Comment on attachment 620856 [details] [diff] [review]
v.2 Use new MigrationUtils.getLocalizedString to handle string overrides (Cu.import in getMigratedData)

Please do file a bug about figuring out where should this code live. I agree that migration.properties dependency is just as bad as MigrationUtils dependency.
Attachment #620856 - Flags: review?(mano) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/86e0e6f1e48b

(In reply to Mano from comment #7)
> Please do file a bug about figuring out where should this code live. I agree
> that migration.properties dependency is just as bad as MigrationUtils
> dependency.

Filed bug 759596
Flags: in-testsuite-
Target Milestone: --- → Firefox 15
https://hg.mozilla.org/mozilla-central/rev/86e0e6f1e48b
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Comment on attachment 620856 [details] [diff] [review]
v.2 Use new MigrationUtils.getLocalizedString to handle string overrides (Cu.import in getMigratedData)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 737381
User impact if declined: history will not be included in the list of preserved data for a reset (which may deter users).
Testing completed (on m-c, etc.): Verified on Nightly 20120530
Risk to taking this patch (and alternatives if risky): Low risk. Using migrationutils API rather than relying directly on strings.
String or UUID changes made by this patch: Deletion of unused 32_firefox.
Attachment #620856 - Flags: approval-mozilla-aurora?
We're looking to approve this, but want to double check with Axel whether anything needs to be done around the string removal before landing.

Comment 12

5 years ago
Per IRC, please don't approve attachment 620856 [details] [diff] [review], it's not doing what it should. Notably, it relies on strings in l10n not being there (32_firefox), which we can't guarantee.
Comment on attachment 620856 [details] [diff] [review]
v.2 Use new MigrationUtils.getLocalizedString to handle string overrides (Cu.import in getMigratedData)

as per Axel's comment above, not approving for aurora.
Attachment #620856 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
My understanding is that this feature is only available through about:support in FF14. SUMO and SUMO users will be the main consumer of this feature for this cycle. Given that, this isn't a blocker in my eyes. Please re-nominate if you disagree.

That being said, if a low risk solution was found (english only?), we'd definitely approve.
tracking-firefox14: + → -
Created attachment 639949 [details] [diff] [review]
v.1 For 14 - Don't depend on existence of string

Still time for beta?

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 737381
User impact if declined: history will not be included in the list of preserved data for a reset (which may deter users).
Testing completed (on m-c, etc.): Not yet as 1st approach landed on m-c already but was denied aurora.
Risk to taking this patch (and alternatives if risky): Low risk. Using specific strings rather than assuming existence of string implies it's migrated.
String or UUID changes made by this patch: Deletion of unused 32_firefox.
Attachment #639949 - Flags: review?(gavin.sharp)
Attachment #639949 - Flags: review?(dolske)
Attachment #639949 - Flags: approval-mozilla-beta?
Created attachment 639950 [details] [diff] [review]
v.3 (m-c/15) Don't depend on the existence of the string (based on IRC discussion)
Attachment #639950 - Flags: review?(gavin.sharp)
Attachment #639950 - Flags: review?(dolske)
Attachment #620856 - Flags: checkin+
Comment on attachment 639949 [details] [diff] [review]
v.1 For 14 - Don't depend on existence of string

Why not just leave the string on beta? Removing it will cause confusion, which seems like more trouble than it's worth.
Attachment #639949 - Flags: review?(gavin.sharp)
Attachment #639949 - Flags: review?(dolske)
Attachment #639949 - Flags: review+
Comment on attachment 639950 [details] [diff] [review]
v.3 (m-c/15) Don't depend on the existence of the string (based on IRC discussion)

Why is this code using "checkbox" as a variable containing a label element? :)

Maybe you should put a note in migration.properties that suggests making sure the hardcoded list in resetProfile.js is up to date?
Attachment #639950 - Flags: review?(gavin.sharp)
Attachment #639950 - Flags: review?(dolske)
Attachment #639950 - Flags: review+
Comment on attachment 639949 [details] [diff] [review]
v.1 For 14 - Don't depend on existence of string

[Triage Comment]
Approving for Beta 14. If we see any major regressions, we'll either disable for our final mozilla-release build or advise SUMO not to use the feature.
Attachment #639949 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
attachment 639950 [details] [diff] [review]: https://hg.mozilla.org/integration/mozilla-inbound/rev/89e5bededa3e
attachment 639949 [details] [diff] [review]: https://hg.mozilla.org/releases/mozilla-beta/rev/2f60b52f5a6c (Tested in local build)
status-firefox14: affected → fixed
status-firefox15: --- → affected
status-firefox16: --- → fixed
tracking-firefox14: - → ?
Comment on attachment 639950 [details] [diff] [review]
v.3 (m-c/15) Don't depend on the existence of the string (based on IRC discussion)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 737381 + l10n fallout from attachment 620856 [details] [diff] [review] which landed on m-c and migrated to aurora.
User impact if declined: bookmarks may be included twice in the list of preserved data for a reset in some locales if they don't delete the 32_firefox string. (Translated once from "Bookmarks" and again from "History and Bookmarks").
Testing completed (on m-c, etc.): Local build and pushed to inbound today.
Risk to taking this patch (and alternatives if risky): Low risk. Using a list of strings rather than assuming existence of string implies it's migrated.
String or UUID changes made by this patch: None
Attachment #639950 - Flags: approval-mozilla-aurora?
tracking-firefox14: ? → +
https://hg.mozilla.org/mozilla-central/rev/89e5bededa3e

Updated

5 years ago
Attachment #639950 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Thanks Gavin and Alex.

https://hg.mozilla.org/releases/mozilla-aurora/rev/b9e356acd3c5
status-firefox15: affected → fixed
Created attachment 648692 [details]
Screenshot of Firefox 15beta3

Verified on Ubuntu 12.04, Mac OS 10.7, Windows 7.

All strings present in 13 are present in 15 as well, but in a different order.
status-firefox15: fixed → verified
I have reproduced this bug on Firefox nightly 15.0a1 on windows(64bit).

this bug is verified as fixed .
Latest Firefox nightly 47.0a1 on windows(64bit)
Build Id:20160304030206
user Agent:Mozilla/5.0 (Windows NT 6.3; WOW64; rv:47.0) Gecko/20100101 Firefox/47.0
You need to log in before you can comment on or make changes to this bug.