Closed
Bug 1173688
Opened 9 years ago
Closed 8 years ago
Password manager sync promo appears when signing in/up for Sync from an iframe
Categories
(Toolkit :: Password Manager, defect)
Tracking
()
People
(Reporter: stomlinson, Assigned: MattN)
References
()
Details
Attachments
(4 files)
Ref [1], [2] The Firefox password manager offers to save passwords for Firefox Accounts/Sync when FxA is embedded in an iframe. The Firefox password manager ignores FxA when embedded in Firefox Accounts, but not when FxA is used for Sync and embedded in an iframe. The new firstrun flow for Fx40 will embed FxA into an iframe, it would be ideal to fix this bug before the Fx40 release. [1] - https://bugzilla.mozilla.org/show_bug.cgi?id=1146904 [2] - https://github.com/mozilla/fxa-content-server/issues/2373 [3] - https://dxr.mozilla.org/mozilla-central/source/toolkit/components/passwordmgr/LoginManagerContent.jsm#659
Comment 1•9 years ago
|
||
I saw this in a custom Fennec build, tried to verify elsewhere and didn't see it again -- presumably because I wasn't hitting the iframe flow. There is a discussion of this particular on dev-fxacct and shane's [3] is the about:accounts specific work-around.
Comment 2•9 years ago
|
||
:bmaggs - do you believe this needs to be done for Fx40?
Flags: needinfo?(wmaggs)
Comment 3•9 years ago
|
||
I can't reproduce. I know from talking to Chris what it is supposed to look like; Shane, could you send me a screenshot? My impression is if it is a quick fix (tiny patch) we should do it now, given the potential to put new Sync users in a signup loop.
Flags: needinfo?(wmaggs) → needinfo?(stomlinson)
Reporter | ||
Comment 4•9 years ago
|
||
Flags: needinfo?(stomlinson)
Reporter | ||
Comment 5•9 years ago
|
||
(In reply to Bill Maggs from comment #3) > I can't reproduce. I know from talking to Chris what it is supposed to look > like; Shane, could you send me a screenshot? > > My impression is if it is a quick fix (tiny patch) we should do it now, > given the potential to put new Sync users in a signup loop. The problem only manifests when signing in to Sync from within an iframe, not the normal about:accounts based flow. No iframe based "sign in to Sync" sites exist yet, the first such site (the firstrun flow) is scheduled to be released with Fx 40. The attached screenshot was created on my local development environment that was used to simulate the firstrun flow when I developed the Fx patch for https://bugzilla.mozilla.org/show_bug.cgi?id=1146904.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → MattN+bmo
Status: NEW → ASSIGNED
Iteration: --- → 41.3 - Jun 29
Points: --- → 3
status-firefox40:
--- → affected
status-firefox41:
--- → affected
Assignee | ||
Comment 6•9 years ago
|
||
Bug 1173688 - Don't show the Sync promo for https://accounts.firefox.com password doorhangers. r=markh
Attachment #8622735 -
Flags: review?(markh)
Assignee | ||
Comment 7•9 years ago
|
||
I added an "origin" property to PopupNotification options since it's part of Shorlander's new doorhanger designs anyways and the information is useful for this hack. See https://www.lucidchart.com/documents/edit/87ab1cc8-e708-49d3-8b91-6e2e6da346fb/1 for an example. I'm passing the whole origin but the front-end can then remove the scheme if it wishes. It's easy to go from a URL/origin to a hostname but not the other way around so that's why I chose passing the origin.
Assignee | ||
Comment 8•9 years ago
|
||
Btw. we use the Target Milestone to indicate the version a fix first lands in, not a future target. The "status-firefoxXY: affected" flags are used for indicating which versions are affected.
Target Milestone: mozilla40 → ---
Reporter | ||
Comment 9•9 years ago
|
||
(In reply to Matthew N. [:MattN] from comment #8) > Btw. we use the Target Milestone to indicate the version a fix first lands > in, not a future target. The "status-firefoxXY: affected" flags are used for > indicating which versions are affected. Thanks Matthew, I'm still learning Bugzilla etiquette.
Assignee | ||
Comment 10•9 years ago
|
||
I just realized that my fix reverts a toolkit change and replaces it with a desktop Firefox one. If there is a similar promo on Android it will also need to be suppressed. Nick, are you saying this is a promo on Fennec? If so, can we get a separate bug for that?
Summary: Password manager offers to save passwords for Firefox Accounts when used to sign in/up for Sync from an iframe → Password manager sync promo appears when signing in/up for Sync from an iframe
Comment 11•9 years ago
|
||
https://reviewboard.mozilla.org/r/11433/#review9803 ::: browser/base/content/urlbarBindings.xml:3074 (Diff revision 1) > + notification.options.origin == "https://accounts.firefox.com") { It would be ideal if we can avoid the hard-coded URL and instead check against one of the fxa prefs. Sadly the state of those prefs isn't great - extracting the origin from identity.fxaccounts.remote.signup.uri is probably ok. ::: toolkit/components/passwordmgr/nsLoginManagerPrompter.js:941 (Diff revision 1) > + origin: login.hostname, it probably wouldn't hurt to add a comment indicating that nsILoginInfo.hostname is actually an origin
Comment 12•9 years ago
|
||
Comment on attachment 8622735 [details] MozReview Request: Bug 1173688 - Don't show the Sync promo for https://accounts.firefox.com password doorhangers. r=markh https://reviewboard.mozilla.org/r/11435/#review9809 Ship It!
Attachment #8622735 -
Flags: review?(markh) → review+
Comment 14•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/1edffee3ccb1
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Assignee | ||
Comment 15•9 years ago
|
||
Comment on attachment 8622735 [details] MozReview Request: Bug 1173688 - Don't show the Sync promo for https://accounts.firefox.com password doorhangers. r=markh Approval Request Comment [Feature/regressing bug #]: Some FxA login attempts won't be from about:accounts anymore in 40 [User impact if declined]: The user may end up in a sync setup loop as a sync promo will push people into setting up sync while they're in the middle of it. [Describe test coverage new/current, TreeHerder]: Manual testing by FxA developers [Risks and why]: Low risk chance of breaking doorhangers but no automated tests failed. [String/UUID change made/needed]: None
Attachment #8622735 -
Flags: approval-mozilla-aurora?
Comment 16•9 years ago
|
||
Comment on attachment 8622735 [details] MozReview Request: Bug 1173688 - Don't show the Sync promo for https://accounts.firefox.com password doorhangers. r=markh New feature, we want that polished.
Attachment #8622735 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Reporter | ||
Comment 18•8 years ago
|
||
Reporter | ||
Updated•8 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Reporter | ||
Comment 19•8 years ago
|
||
This seems to have regressed, I am again being asked to save my FxA password from about:accounts and the firstrun page.
Reporter | ||
Comment 20•8 years ago
|
||
Assignee | ||
Comment 21•8 years ago
|
||
Note that you shouldn't re-open fixed Fx bugs, especially more than a day or two after landing as we use the bug fields to track the version a fix occurred in. Fixing a bug multiple times in one BMO bug makes it hard to understand the history of fixes. The usual process is to file a new bug blocking the original fix. (In reply to Shane Tomlinson [:stomlinson] from comment #19) > This seems to have regressed, I am again being asked to save my FxA password > from about:accounts and the firstrun page. Nope, see the bug summary. We decided it's fine to offer to save the password but simply not show the "sync promo" promoting sync while you're logging into sync. You're screenshot doesn't show the footer of the panel where the promo would be so I can't tell if the promo was there.
Status: REOPENED → RESOLVED
Closed: 9 years ago → 8 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 22•8 years ago
|
||
(In reply to Matthew N. [:MattN] from comment #21) > Note that you shouldn't re-open fixed Fx bugs, especially more than a day or > two after landing as we use the bug fields to track the version a fix > occurred in. Fixing a bug multiple times in one BMO bug makes it hard to > understand the history of fixes. The usual process is to file a new bug > blocking the original fix. Thanks for the etiquette info, I did not know. > > (In reply to Shane Tomlinson [:stomlinson] from comment #19) > > This seems to have regressed, I am again being asked to save my FxA password > > from about:accounts and the firstrun page. > > Nope, see the bug summary. We decided it's fine to offer to save the > password but simply not show the "sync promo" promoting sync while you're > logging into sync. You're screenshot doesn't show the footer of the panel > where the promo would be so I can't tell if the promo was there. ckarlof gave us the full history last night, everything makes sense now. The dropdown did not show the Sync promo. Thanks Matt!
You need to log in
before you can comment on or make changes to this bug.
Description
•