+++ This bug was initially created as a clone of Bug #1175055 +++ +++ This bug was initially created as a clone of Bug #917961 +++ Some crashes are still occuring after bug 1175055, because signature is #5 crash for version 38.1.0 https://crash-stats.mozilla.com/report/list?product=Thunderbird&range_value=7&range_unit=days&date=2015-07-21&signature=mozilla%3A%3Adom%3A%3Aworkers%3A%3AGetCurrentThreadJSContext%28%29&version=Thunderbird%3A38.1.0#tab-comments a couple examples: bp-80cb63b8-8104-4ea9-83ed-6a3632150719 bp-08c5ca60-c59d-4476-ae66-da8ac2150714

I thought the Outlook migration at initial install of TB was disabled by bug 1175055. I won't have access to the machine with Outlook I used for testing until 13th Aug. 2015.

I am certainly fielding questions in support about it not working and being disabled. So we must have disabled it.

The outlook crashes are currently ranked #9 and #10 for 38.1.0, so still very highly ranked indeed. The feedback I am getting by private email from users is they are using Tools > Import. So there must be a code path that is still enabled. https://crash-stats.mozilla.com/search/?product=Thunderbird&version=38.1.0&date=%3E2015-07-01&user_comments=~import&_facets=signature&_columns=date&_columns=signature&_columns=version&_columns=build_id&_columns=user_comments#crash-reports

As I said in comment #1: I can't work on this until 13th Aug. 2015. Do these users use "Import Everything" or do they import specific items? As far as I can see on my travel laptop, all Outlook and Eudora import options are disabled. We could speed this up if we had a screenshot of what they are doing.

Several users reported they used "Import Everything" which offered outlook. One of those observed that the outlook choice was "selected" but was also "disabled". I'm not sure what that means. Attempting to get clarifications from users.

Yes, exactly!! Outlook is offered, selected (if it's the only choice) and disabled (greyed out). From there, it should be *impossible* to click "Next", since no valid option was selected. Code: #ifdef XP_WIN <radio id="oexpress" label="&importFromOExpress.label;" accesskey="&importFromOExpress.accesskey;"/> <radio id="outlook" label="&importFromOutlook.label;" accesskey="&importFromOutlook.accesskey;" tooltiptext="Currently disabled due to bug 1175055" disabled="true"/> #endif I won't repeat when I can look at it again.

Although the option is disabled, you can still press "Next". I could swear I tested that in bug 1175055, as far as I remember you couldn't press "Next". I'll look into it.

You can land this on TB 38.x to fix the crashes. Note to the reviewer: This bit: + if (!this._migrator) { + return; + } has nothing to do with the fix. I noticed a JS error (dereferenced null) on "Import Nothing", so I fixed it. If one day we fix the Outlook and Eudora import, this patch does *NOT* have to be rolled back. The only one that needs to be rolled back is in bug 1175055.

I don't know how much testing reviewers do. You can test this patch without Outlook and Eudora. Just install SeaMonkey on your machine, then edit comm-central/mail/components/migration/content/migration.xul and add tooltiptext="Currently disabled due to bug 1175055" disabled="true" to the SeaMonkey item.

"I don't know how much testing reviewers do. You can test this patch without Outlook and Eudora." Even with Outlook, the import code AFAICT only supports Outlook 98 and Outlook 2003 - and I don't have those old Outlooks! Hard to get too, now that Technet is discontinued so there is no affordable official source of Microsoft test software. Anyway I did my own test (by changing the registry key location to point to the Outlook that I do have installed) and can confirm that I can generate a crash without this patch, but the crash is prevented (because the "Outlook" entry though found is disabled with the patch, but enabled without) and the "Next" is disabled with this patch, and enabled without.

Comment on attachment 8645387 [details] [diff] [review] Fix up wizard to deal with disabled items. Don't select them and don't allow "Next" with no valid selection. Review of attachment 8645387 [details] [diff] [review]: ----------------------------------------------------------------- r+ with suggested revisions. I'll also post the patch that I tested that include those revisions, you can land that patch if you also agree with the revisions. ::: mail/components/migration/content/migration.js @@ +94,5 @@ > } > group.selectedItem = this._source == "" ? firstNonDisabled : document.getElementById(this._source); > + > + if (!firstNonDisabled) { > + var nextButton = this._wiz.getButton("next"); Generally the "next" button is controlled by the wizard, and it is not good to look into the wizard and change its UI elements. That's what canAdvance is for, so please use that. I also prefer that canAdvance be disabled at the top of this, and only enabled when we have firstNonDisabled so that we default to do nothing. Better if there are unknown errors.

I think we should take this for 38.2.0

Comment on attachment 8645387 [details] [diff] [review] Fix up wizard to deal with disabled items. Don't select them and don't allow "Next" with no valid selection. I was looking for a solution that involved the wizard controls, but couldn't figure it out. Kent's solution works and is much cleaner.

Comment on attachment 8646080 [details] [diff] [review] With RKJ revision Review of attachment 8646080 [details] [diff] [review]: ----------------------------------------------------------------- Looks good and works! Thanks for improving on my initial suggestion.

Comment on attachment 8646080 [details] [diff] [review] With RKJ revision [Approval Request Comment] User impact if declined: Outlook import enabled and crashing. Testing completed (on c-c, etc.): Yes, both Jorg and Kent tested in different ways. Risk to taking this patch (and alternatives if risky): Small wizard tweak, no risk.

Comment on attachment 8646080 [details] [diff] [review] With RKJ revision https://hg.mozilla.org/releases/comm-esr38/rev/9653ee53a6fa

Comment on attachment 8646080 [details] [diff] [review] With RKJ revision http://hg.mozilla.org/releases/comm-aurora/rev/446ee8397c77 https://hg.mozilla.org/releases/comm-beta/rev/4f1d6e940fc4