1403402 - Import dialog appears as well as the import message on New Tab

Status: VERIFIED FIXED

(Firefox :: Migration, defect, P1)

Description

[Jennifer Davidson (she/her) [:jdavidson]]

Firefox Build: Firefox Beta 57.0b3 (64-bit)
OS: Windows 10 

A new user downloads and installs Firefox Beta. The import dialog box should NOT appear on startup, but it does. The import dialog box should NOT appear because there is an import message on New Tab. 

See the following video clip from usertesting.com: http://www.usertesting.com/c/82008db0-7a47-4400-bc1e-8b9aab5f022d?note_id=clip-1384327&shared=qTpKaT-j

Comment 1

[Fischer [:Fischer]]

(In reply to Jennifer Davidson (she/her) from comment #0)
> Firefox Build: Firefox Beta 57.0b3 (64-bit)
> OS: Windows 10 
> 
> A new user downloads and installs Firefox Beta. The import dialog box should
> NOT appear on startup, but it does. The import dialog box should NOT appear
> because there is an import message on New Tab. 
> 
> See the following video clip from usertesting.com:
> http://www.usertesting.com/c/82008db0-7a47-4400-bc1e-8b9aab5f022d?note_id=clip-1384327&shared=qTpKaT-j
We get profile count at [1] and then set `gDoMigration` to true at [2], which means do profile migration when finding no profile(that's the case for a fresh install). The solution should set `gDoMigration` to false when finding no profile. Building a test build... 
 
[1] http://searchfox.org/mozilla-central/rev/f54c1723befe6bcc7229f005217d5c681128fcad/toolkit/xre/nsAppRunner.cpp#2500
[2] http://searchfox.org/mozilla-central/rev/f54c1723befe6bcc7229f005217d5c681128fcad/toolkit/xre/nsAppRunner.cpp#2593

Comment 2

[Fischer [:Fischer]]

Attachment: ScreenCapture_import_on_fresh_install.mp4

Comment 3

[Fischer [:Fischer]]

Attachment: ScreenCapture_no_import_on_fresh_install.mp4

Comment 4

[Fischer [:Fischer]]

Attachment: Bug 1403402 - Should not migrate profile for a fresh install,

Review commit: https://reviewboard.mozilla.org/r/184458/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/184458/

Comment 5

[Fischer [:Fischer]]

(In reply to Fischer [:Fischer] from comment #4)
> Created attachment 8913168 [details]
> Bug 1403402 - Should not migrate profile for a freash install
> 
> Review commit: https://reviewboard.mozilla.org/r/184458/diff/#index_header
> See other reviews: https://reviewboard.mozilla.org/r/184458/
The video[1] shows the import dialog appears during a fresh installation.
As the comment 1, simply setting `gDoMigration` to false when finding no profile would work. With the patch no import dialog would show during a fresh installation[2]. Running Try...

[1] attachment 8913165 [details]: ScreenCapture_import_on_fresh_install.mp4
[2] attachment 8913166 [details]: ScreenCapture_no_import_on_fresh_install.mp4

Comment 6

[Fischer [:Fischer]]

Comment on attachment 8913168 [details]
Bug 1403402 - Should not migrate profile for a fresh install,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184458/diff/1-2/

Comment 7

[Fischer [:Fischer]]

Comment on attachment 8913168 [details]
Bug 1403402 - Should not migrate profile for a fresh install,

https://reviewboard.mozilla.org/r/184458/#review189786

::: commit-message-e1f66:1
(Diff revision 2)
> +Bug 1403402 - Should not migrate profile for a fresh install, r?MattN

Please see the videos [1][2] for the test build in action and [3] for Try, thanks



[1] attachment 8913165 [details]: ScreenCapture_import_on_fresh_install.mp4
[2] attachment 8913166 [details]: ScreenCapture_no_import_on_fresh_install.mp4
[3] https://treeherder.mozilla.org/#/jobs?repo=try&revision=edfe09fe2a21

Comment 8

[Fischer [:Fischer]]

Comment on attachment 8913168 [details]
Bug 1403402 - Should not migrate profile for a fresh install,

https://reviewboard.mozilla.org/r/184458/#review189790

::: commit-message-e1f66:1
(Diff revision 2)
> +Bug 1403402 - Should not migrate profile for a fresh install, r?MattN

Sorry, to be clear, the video [1] is the build without the patch, which is the failure case. The vidoe [2] is the build with the patch, which is the successful case.

Comment 9

[Matthew N. [:MattN]]

Comment on attachment 8913168 [details]
Bug 1403402 - Should not migrate profile for a fresh install,

So the patch achieves what you want but I think we only want this change for activity stream users. I tried looking on Trello and firefox-dev updates but I'm not 100% sure that activity stream with this message are shipping in 57 to all eligible users. Can you confirm this? Also, I believe Activity Stream is only targeting specific countries so how will users in other countries import? Will the import notification still show outside those countries?

Comment 10

[Ed Lee :Mardak]

Activity Stream is on by default for all 57 users. This is for both about:home and about:newtab. The only part of Activity Stream that is locale/region specific is the stories/Pocket section (English languages in US/CA and German languages in DE).

Comment 11

[Fischer [:Fischer]]

Comment on attachment 8913168 [details]
Bug 1403402 - Should not migrate profile for a fresh install,

Hi Matt,
As comment 10, the AS about:newtab/about:home are on for all users. Only the stories/Pocket section is locale/region specific. All fresh install users would see the manual migration option.

Comment 12

[Matthew N. [:MattN]]

Comment on attachment 8913168 [details]
Bug 1403402 - Should not migrate profile for a fresh install,

https://reviewboard.mozilla.org/r/184458/#review191210

Thanks for checking into AS release plans.

::: toolkit/xre/nsAppRunner.cpp:2593
(Diff revision 2)
>      }
>    }
>  #endif
>  
>    if (!count) {
> -    gDoMigration = true;
> +    // For a freash install, we would like to let users decide

typo: fresh

::: toolkit/xre/nsAppRunner.cpp:2594
(Diff revision 2)
> +    // to do profile migration on their own later after using so
> +    // set `gDoMigration` to false

remove the "so set `gDoMigration` to false" part since that's obvious from the line below

Comment 13

[Fischer [:Fischer]]

Comment on attachment 8913168 [details]
Bug 1403402 - Should not migrate profile for a fresh install,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184458/diff/2-3/

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/19185f801cdd
Should not migrate profile for a fresh install, r=MattN

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/19185f801cdd

Comment 16

[Fischer [:Fischer]]

Hi Cristian,

This bug is important. That would be great you could verify it as soon as the nightly build with it comes out, thanks.
There are 2 verifications required:

Verification #1:
1. Install Chrome or IE if there wasn't, browsing some websites, saving some bookmarks on it.
2. Make sure no Firefox and no old Firefox profile on the system. If there was any, remove all entirely so that the system was like never installed with Firefox.
3. Download the Nightly build with the bug's patch
4. On Windows, install the Nightly. On Linux/Mac, open the Nightly
5. Make sure no dialog asking to import bookmark/history from other browsers, like Chrome/IE, shows during installation or the 1st openning
6. The Nightly runs normally

Verification #2:
Please follow bug 1376558 comment 13. This is to make sure no regression on bug 1376558 from this bug. I have tested and should be ok but please still verify it.

Comment 17

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

I verified this issue on Windows 10 x64 using Fx 58.0a1 (build ID: 20171005100211) and following steps from comment 16. I can confirm this issue is fixed.

Comment 18

[Fischer [:Fischer]]

Comment on attachment 8913168 [details]
Bug 1403402 - Should not migrate profile for a fresh install,

Approval Request Comment
[Feature/Bug causing the regression]: 
Bug 1403402

[User impact if declined]:
For a fresh install, user would see the history/bookmark import dialog during installation. However, in about:home/about:newtab there is another history/bookmark import option displayed to user.
Without this fix, 
1. user might feel confusing why Firefox keeps asking for importing history/bookmark from other browser.
2. the import dialog during installation would bring negative impact to installation UX
3. some unexpected issue might happen if user imported history/bookmark from other browser twice (once during installation and once in about:home/about:newtab page).

[Is this code covered by automated tests?]:
No. This code simply skip import dialog during installation

[Has the fix been verified in Nightly?]:
Yes

[Needs manual test from QE? If yes, steps to reproduce]: 
Yes, please follow Bug 1403402 comment 16

[List of other uplifts needed for the feature/fix]:
No

[Is the change risky?]:
No. This code simply skip import dialog during installation, which doesn't affect Firefox's running. User still could use the import option on about:home/about:newtab page to do import if user would like after using Firefox.

[Why is the change risky/not risky?]:
Low risk. This patch only sets one variable controlling import dialog to false so as to skip the import dialog during installation. Firefox still runs normally without importing data from other browsers. However, some unexpected issue might happen if user imported twice.

[String changes made/needed]:
No

Comment 19

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8913168 [details]
Bug 1403402 - Should not migrate profile for a fresh install,

Fixes a recent regression, Beta57+

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3e7dd28c3883

Comment 21

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

I verified this issue using Fx 57.0b8 (build ID: 20171005100211)following steps from comment 16 on Windows 10 x64 , Ubuntu 14.04 LTS and mac OS X . I can confirm this issue is fixed.