Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

WIP Bug 897600: Hook up the Firefox Accounts intro screen in the FTE #14618

Closed
wants to merge 1 commit into from
Closed

WIP Bug 897600: Hook up the Firefox Accounts intro screen in the FTE #14618

wants to merge 1 commit into from

Conversation

shane-tomlinson
Copy link

@@ -351,6 +359,17 @@ var UIManager = {
settings.createLock().set(cset);
},

createFirefoxAccount: function ui_createFirefoxAccount() {
var showResponse = function showResponse(response) {
alert('Success: ' + JSON.stringify(response));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the alert

@shane-tomlinson
Copy link
Author

@borjasalguero - updated!

@borjasalguero
Copy link
Contributor

@shane-tomlinson We would need some tests here! Please take a look!

@shane-tomlinson
Copy link
Author

@borjasalguero - can somebody else, perhaps @6a68 take this over? I have very little bandwidth at the moment.

@jaredhirsch
Copy link
Contributor

I'm happy to take this over.

@borjasalguero can you be a bit more specific about what tests you'd like to see?

@SamPenrose do we have a bugzilla ticket in the tree for FTE work? I'm not sure how we want to prioritize this, I'll defer to you.

@SamPenrose
Copy link
Contributor

@6a68 That would be awesome. The 1.4 version of 897600 is https://bugzilla.mozilla.org/show_bug.cgi?id=949063 . I am going to announce the big Bugzilla cleanup at the Thursday 8am PST meeting with TEF; assuming no objections I'll mark 897600 as dupe then.

@borjasalguero
Copy link
Contributor

Relaunching Travis!

@borjasalguero
Copy link
Contributor

@SamPenrose Sam, which bug is going to be the one for landing this feature? https://bugzilla.mozilla.org/show_bug.cgi?id=949063? If we are going to change to this we need to assign the bug properly, upload the patch and ask to review again. @shane-tomlinson Could you take a look? We have just landed FxA Module in master! :)! \m/

@SamPenrose
Copy link
Contributor

@borjasalguero I would like to do whatever is simplest. Can we land this under the current bug, then update the new bug as needed?

@jaredhirsch
Copy link
Contributor

@SamPenrose @borjasalguero I didn't realize changing the bug number would require another review; I've reopened 897600 to make life simpler :-)

@borjasalguero
Copy link
Contributor

@shane-tomlinson This patch needs to solve as well the following:
https://bugzilla.mozilla.org/show_bug.cgi?id=935602

@SamPenrose
Copy link
Contributor

@borjasalguero just to be clear, do you think we must fix 935602 before landing this? If so, do you have any ideas how @6a68 can implement the disabling of hints?

@jaredhirsch
Copy link
Contributor

@SamPenrose fixing 935602 should be easy, we should be able to just add x-inputmode="verbatim" which should work[1][2].

[1] http://www.otsukare.info/2013/12/03/form-spelling-feature
[2] https://groups.google.com/d/topic/mozilla.dev.webapi/se0h7PUrQOY/discussion

@borjasalguero
Copy link
Contributor

@6a68 Yep! :) I would like to add this fix to this patch. As well we need to fix the test which is failing right now. I dont know if @shane-tomlinson has bandwith here, Jared could you sync with Shane? Maybe you can create a PR for this small fix and for making Travis become to green! Wdyt?

@jaredhirsch
Copy link
Contributor

@borjasalguero yes, i'm talking with shane and i'll just push to this branch with the extra tests and the x-inputmode fix

@jaredhirsch
Copy link
Contributor

arrrgh. I can't reproduce the travis failures locally, I'm guessing the timeout is because of travis VM slowness? @borjasalguero or @ferjm, would you mind restarting the travis job?

@jaredhirsch
Copy link
Contributor

auswerk restarted the travis job for me, it failed a second time. I can't repro the failures locally, working on that now.

@jaredhirsch
Copy link
Contributor

I really have no idea how to reproduce those failures locally. any advice @ferjm / @borjasalguero ?

@jaredhirsch
Copy link
Contributor

I figured out how to reproduce the error locally--it's not spurious. working on a fix.

@jaredhirsch
Copy link
Contributor

Sweet! Travis says tests passed, and I've added the fix for 935602. @borjasalguero does this look good to merge?

@borjasalguero
Copy link
Contributor

@6a68 Jared, could you squash all commits into one? Thanks!

@jaredhirsch
Copy link
Contributor

@borjasalguero Sure, no problem. I've rebased against today's upstream and squashed everything into one commit.

@jaredhirsch
Copy link
Contributor

This PR was getting a bit stale, so I've rebased it again against 1/29 master.

  * Also fixes bug 935602 - disable autocorrect on inputs in firefox accounts
    system app
@jaredhirsch
Copy link
Contributor

Rebased again, this time against 2/19 master

@mozilla-autolander-deprecated
Copy link
Contributor

This pull request has been closed due to tree stability issues. Please rebase and re-open the pull request if you still need to land this. Ensure the gaia-try run is green before landing. Sorry for any inconvenience.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
5 participants