653335 - Sync setup wizard triggers Change Master Password dialog

Status: VERIFIED FIXED

(Firefox :: Sync, defect, P2)

Description

[Himanshu]

User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:5.0a2) Gecko/20110427 Firefox/5.0a2
Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:5.0a2) Gecko/20110427 Firefox/5.0a2

I installed Aurora and then clicked on Set Up Sync

On clicking on Connect I got this message : http://dl.dropbox.com/u/7456129/Firefox/sync.jpg
which is abnormal

However, when I closed it and then again tried to connect, I was able to do.

Reproducible: Always

Comment 1

[Himanshu]

(In reply to comment #0)
> User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:5.0a2) Gecko/20110427
> Firefox/5.0a2
> Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:5.0a2) Gecko/20110427
> Firefox/5.0a2
> 
> I installed Aurora and then clicked on Set Up Sync
> 
> On clicking on Connect I got this message :
> http://dl.dropbox.com/u/7456129/Firefox/sync.jpg
> which is abnormal
> 
> However, when I closed it and then again tried to connect, I was able to do.
> 
> Reproducible: Always

Has anyone checked this?

Comment 2

[Philipp von Weitershausen [:philikon]]

Couple of questions:

* Can you reproduce this reliably?

* Do you use a master password?

* What add-ons do you have installed?

Comment 3

[Himanshu]

1.Yes I have tried this two times upon first install.

2.No don't use a master password

3.Nothing special.  Same addons were there when I tried on FF4 and there it worked in first attempt only.

Comment 4

[Philipp von Weitershausen [:philikon]]

(In reply to comment #3)
> 3.Nothing special.  Same addons were there when I tried on FF4 and there it
> worked in first attempt only.

Please copy & paste the list of extensions from about:support.

Comment 5

[Himanshu]

  Application Basics

        Name
        Firefox

        Version
        5.0a2

        User Agent
        Mozilla/5.0 (Windows NT 5.1; rv:5.0a2) Gecko/20110429 Firefox/5.0a2

        Profile Directory

          Open Containing Folder

        Enabled Plugins

          about:plugins

        Build Configuration

          about:buildconfig

  Extensions

        Name

        Version

        Enabled

        ID

        Add-on Compatibility Reporter
        0.8.3
        true
        [e-mail]

        Feedback
        1.0.9
        true
        [e-mail]

        IDM CC
        7.1.8
        true
        [e-mail]

        Java Quick Starter
        1.0
        true
        [e-mail]

        Microsoft .NET Framework Assistant
        0.0.0
        true
        {20a82645-c095-46ed-80e3-08825760534b}

  Modified Preferences

      Name

      Value

        accessibility.typeaheadfind.flashBar
        0

        browser.places.smartBookmarksVersion
        2

        browser.startup.homepage
        about:home

        browser.startup.homepage_override.buildID
        20110429042003

        browser.startup.homepage_override.mstone
        rv:5.0a2

        browser.tabs.warnOnClose
        false

        extensions.checkCompatibility
        false

        extensions.checkCompatibility.3.6
        false

        extensions.checkCompatibility.3.6b
        false

        extensions.checkCompatibility.3.6p
        false

        extensions.checkCompatibility.3.6pre
        false

        extensions.checkCompatibility.3.7a
        false

        extensions.checkCompatibility.4.0
        false

        extensions.checkCompatibility.4.0b
        false

        extensions.checkCompatibility.4.0p
        false

        extensions.checkCompatibility.4.0pre
        false

        extensions.checkCompatibility.4.2
        false

        extensions.checkCompatibility.4.2a
        false

        extensions.checkCompatibility.4.2b
        false

        extensions.checkCompatibility.4.2p
        false

        extensions.checkCompatibility.4.2pre
        false

        extensions.checkCompatibility.5.0
        false

        extensions.checkCompatibility.5.0a
        false

        extensions.checkCompatibility.5.0b
        false

        extensions.checkCompatibility.5.0p
        false

        extensions.checkCompatibility.5.0pre
        false

        extensions.checkCompatibility.6.0
        false

        extensions.checkCompatibility.6.0a
        false

        extensions.checkCompatibility.7.0
        false

        extensions.checkCompatibility.7.0a
        false

        extensions.checkCompatibility.8.0
        false

        extensions.checkCompatibility.8.0a
        false

        extensions.lastAppVersion
        5.0a2

        gfx.blacklist.direct2d
        2

        gfx.blacklist.layers.direct3d10
        2

        gfx.blacklist.layers.direct3d10-1
        2

        network.cookie.prefsMigrated
        true

        places.database.lastMaintenance
        1304092977

        places.history.expiration.transient_current_max_pages
        15814

        privacy.clearOnShutdown.cookies
        false

        privacy.clearOnShutdown.downloads
        false

        privacy.clearOnShutdown.formdata
        false

        privacy.clearOnShutdown.history
        false

        privacy.clearOnShutdown.sessions
        false

        privacy.sanitize.migrateFx3Prefs
        true

        privacy.sanitize.timeSpan
        0

        security.warn_viewing_mixed
        false

  Graphics

        Adapter Description
        Intel(R) 82915G/GV/910GL Express Chipset Family

        Vendor ID
        8086

        Device ID
        2582

        Adapter RAM
        Unknown

        Adapter Drivers
        igxprd32

        Driver Version
        6.14.10.4764

        Driver Date
        1-13-2007

        Direct2D Enabled
        false

        DirectWrite Enabled
        false (0.0.0.0, font cache n/a)

        WebGL Renderer
        Google Inc. -- ANGLE -- OpenGL ES 2.0 (ANGLE 0.0.0.611)

        GPU Accelerated Windows
        0/1

Comment 6

[Philipp von Weitershausen [:philikon]]

Tracy says he has seen this but could never reproduce it. Marking as confirmed.

Comment 7

[Tracy Walker [:tracy]]

I can not reproduce this on mozilla-central. But I can reproduce it reliably on Aurora. :-(

STR:

1) start Aurora with a new profile
2) Go to sync setup
3) Click either of the account setup options

Tested result:
Master Password reset dialog appears

Expected result:
Taken to appropriate setup path

Notes: 
- If you cancel the MP dialog, retrying one of the setup options fires the MP dialog again.
- If you click OK to the MP dialog, then OK to the alert message, Setup behaves fine.

Comment 8

[Tracy Walker [:tracy]]

The bug is not present in Fx5, Fx5.0.1 nor in Fx6b4.

The bug remained in aurora past the merge to beta on 5/24. Oddly, the Fx5betas do not have the bug.  How did the bug in aurora not get merged at that point?

I'll try to find when it regressed in aurora and if it ever was a bug on mozilla-central

Comment 9

[Tracy Walker [:tracy]]

This regressed in the Aurora builds on 20110412 (20110411 works). 
Coincidentally, that was the same time frame for branding change from Minefield to Aurora.  It seems impossible branding could be in play here.  But it's eerily odd, since the bug magically disappears when merged to Betas Fx branding.

Not even going to bother with m-c at this point, as I am guessing it never existed there.

Comment 10

[Philipp von Weitershausen [:philikon]]

(In reply to comment #9)
> This regressed in the Aurora builds on 20110412 (20110411 works). 
> Coincidentally, that was the same time frame for branding change from
> Minefield to Aurora.  It seems impossible branding could be in play here. 

It probably was the day they migrated whatever was in m-c to aurora, so not just the branding changed, but they also pulled in a lot more changes. This is just suspicion, but it'd be interesting to see if the problem exists on m-c of that day.

Comment 11

[Tracy Walker [:tracy]]

(In reply to comment #9)
> This regressed in the Aurora builds on 20110412 (20110411 works). 
I made a minor error here transposing from my regression window hunting notes. 20110413 was the first aurora build with this bug. (which coincides with the branding change) Note: the first two aurora builds (20110411 and 20110412) were branded Minefield.  

> 
> It probably was the day they migrated whatever was in m-c to aurora, so not
> just the branding changed, but they also pulled in a lot more changes. This
> is just suspicion, but it'd be interesting to see if the problem exists on
> m-c of that day.

Yes, that does make sense. However, the bug does not exist on mozilla-central on 20110412 nor does it exist on m-c build of 20110413. So something must have landed directly on aurora (and not m-c) during that window.

But that doesn't explain why the bug wasn't carried forward when Aurora got merged to Beta.

Comment 12

[Philipp von Weitershausen [:philikon]]

I can confirm that I saw this on an older Aurora build today. I then upgraded Aurora to the latest build from today and the problem went away. Are you still seeing this, Tracy?

I suspect something in the PSM regressed on Aurora and then got backed out again.

Comment 13

[Tracy Walker [:tracy]]

(In reply to Philipp von Weitershausen [:philikon] from comment #12)
> I can confirm that I saw this on an older Aurora build today. I then
> upgraded Aurora to the latest build from today and the problem went away.
> Are you still seeing this, Tracy?
> 
> I suspect something in the PSM regressed on Aurora and then got backed out
> again.

When I get a chance I'll look into it. It would be good to know what fix/backout caused the behavior to change.

Comment 14

[Tracy Walker [:tracy]]

I still this with Aurora nightly build of 20110815.

Comment 16

[Mike Connor [:mconnor]]

I can still repro on current Aurora, only on very first invocation in a new profile.  I suspect it remains broken.

Comment 17

[Mike Connor [:mconnor]]

followup: hello corner case!

culprit is calling encryptString in ensureMPUnlocked blindly, which prompts for a string to create a token, since one doesn't exist.  Why?  Because we were relying on side-effects (bad!).

What's going on is about:home as default homepage doesn't cause pwmgr to init, so there's no token init happening before we get to the wizard.  Loading a web page causes init, which is why this is a change (and why this doesn't happen on Nightly builds).  Canceling the wizard also calls pwmgr APIs, which is why subsequent wizard invocations work normally.

Solution is simple: only call encryptString if the MP is actually locked.  Handily, we have a helper function for that already, so it's a two-line patch.

Comment 18

[Richard Newman [:rnewman]]

Attachment: Proposed patch. v1

Verified by hand. Not sure if we can write any automated UI tests for this, or if it's worth the time investment…

Steps to validate:

  * ./firefox-bin -no-remote -P some-new-profile-1 -safe-mode about:config
    -- This ensures that the PW infra doesn't get initialized by a new page, even on Nightly.
  * Tools > Set Up Sync
  * Click "Create"
  * Observe that MP dialog does not appear.

Comment 19

[Philipp von Weitershausen [:philikon]]

Comment on attachment 569776 [details] [diff] [review]
Proposed patch. v1

This makes sense to me.

Comment 20

[Richard Newman [:rnewman]]

https://hg.mozilla.org/services/services-central/rev/bfa27e1c3bae

See Comment 18 for STR.

Comment 21

[Richard Newman [:rnewman]]

Comment on attachment 569776 [details] [diff] [review]
Proposed patch. v1

Requesting approval for Aurora. Very small and safe patch that avoids a confusing Master Password prompt if you attempt to set up Sync before visiting any pages.

Comment 22

[Philipp von Weitershausen [:philikon]]

Comment on attachment 569776 [details] [diff] [review]
Proposed patch. v1

All versions since Firefox 4 are affected, so ideally this should be in Beta too. Like comment 21 says, it's a very small and safe patch.

Comment 23

[LegNeato]

Comment on attachment 569776 [details] [diff] [review]
Proposed patch. v1

[Triage Comment]

Approved for aurora9. Not critical enough for beta so denying there.

Comment 24

[Richard Newman [:rnewman]]

Pushed to Aurora, thanks LegNeato.

http://hg.mozilla.org/releases/mozilla-aurora/rev/9748abf80291

Will reach m-c in our next scheduled train merge later this week.

Comment 25

[Tracy Walker [:tracy]]

verified in aurora build of Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:9.0a2) Gecko/20111101 Firefox/9.0a2

I still don't don't see it on nightly (s-c)  but never did see it there (because of the nightly first run page)

Comment 26

[Philipp von Weitershausen [:philikon]]

https://hg.mozilla.org/mozilla-central/rev/bfa27e1c3bae

Comment 27

[Mihaela Velimiroviciu (:mihaelav)]

Mozilla/5.0 (Windows NT 6.1; rv:9.0) Gecko/20100101 Firefox/9.0
Mozilla/5.0 (Windows NT 6.1; rv:10.0a2) Gecko/20111116 Firefox/10.0a2
Mozilla/5.0 (Windows NT 6.1; rv:11.0a1) Gecko/20111116 Firefox/11.0a1

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:9.0) Gecko/20100101 Firefox/9.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:10.0a2) Gecko/20111116 Firefox/10.0a2
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0a1) Gecko/20111117 Firefox/11.0a1

Mozilla/5.0 (Windows NT 5.1; rv:9.0) Gecko/20100101 Firefox/9.0
Mozilla/5.0 (Windows NT 5.1; rv:10.0a2) Gecko/20111110 Firefox/10.0a2
Mozilla/5.0 (Windows NT 5.1; rv:11.0a1) Gecko/20111110 Firefox/11.0a1

Mozilla/5.0 (X11; Linux x86_64; rv:9.0) Gecko/20100101 Firefox/9.0
Mozilla/5.0 (X11; Linux x86_64; rv:10.0a2) Gecko/20111117 Firefox/10.0a2
Mozilla/5.0 (X11; Linux x86_64; rv:11.0a1) Gecko/20111117 Firefox/11.0a1

Verified on the above builds using the STR from comment #18 and issue is not reproducible: MP is not prompted when creating a sync account.