Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Fix Chrome autofill #4228

Closed
wants to merge 21 commits into from
Closed

Fix Chrome autofill #4228

wants to merge 21 commits into from

Conversation

mattbk
Copy link
Contributor

@mattbk mattbk commented Dec 9, 2016

Closes #4041. I’m guessing this will take several iterations to get right— testing locally is difficult because Chrome won’t autofill unless the connection is secure. Yes, but we get a popup telling us it's insecure, and that's almost as good.

@mattbk
Copy link
Contributor Author

mattbk commented Jan 5, 2017

I wonder if a self-signed certificate would work?

@mattbk
Copy link
Contributor Author

mattbk commented Jan 6, 2017

I am unable to create a self-signedcertificate that works for me, but I think that's beside the point. The behavior is the same, even with suggested changes: No autofill message comes up until typing at least two numbers and then hitting backspace.

I will research further.

@mattbk
Copy link
Contributor Author

mattbk commented Jan 6, 2017

Aha! If I disable Javascript, it wakes right up (previously noted).

@mattbk
Copy link
Contributor Author

mattbk commented Jan 6, 2017

  • I can still submit with JS off.
  • I still get popup errors on submit when I don't have an expiration date or CVV.
  • I do not get an error if the number is bad or doesn't match a known merchant format ("Credit card type is not accepted by this merchant account...").

Cf. http://stackoverflow.com/questions/7248488/what-would-cause-chrome-autofill-to-stop-working

Hmmmm, related? We're using Braintree's validation in routes.js: https://bugs.chromium.org/p/chromium/issues/detail?id=585505

Or as simple as not working at all for Braintree? Credit Card Autofill Does Not Work.

@mattbk
Copy link
Contributor Author

mattbk commented Jan 6, 2017

Doesn't seem related to the ZIP and hidden Braintree fields being non-cc fields (as suggested somewhere). I'm going to bed.

@mattbk
Copy link
Contributor Author

mattbk commented Jan 7, 2017

Even if I drop all the way down to

        <form id="credit-card">
                <input id="card_number" required autocomplete="cc-number" />
            <button class="selected larger" id="save" type="submit">{{ _("Save") }}</button>
        </form>

the behavior is the same. Is there some master JS that is messing with all form data?

@mattbk
Copy link
Contributor Author

mattbk commented Jan 7, 2017

It's this part of the Gratipay.routes.cc.init function in routes.js:

    Gratipay.routes.cc.formatInputs(
        $('#card_number'),
        $('#expiration_month'),
        $('#expiration_year'),
        $('#cvv')
    );

If I change or remove it (or just the #card_number form input ID, the autocomplete works...but it won't let me put the right values into MM/DD or CVV. Sometimes it won't even let me put two digits in the date fields, just one. Weird.

@mattbk
Copy link
Contributor Author

mattbk commented Jan 7, 2017

Well, it's this one: Gratipay.routes.cc.formatInputs().

Aha! If I just don't run the CC number through that function, autocomplete works.

Let's commit that and see.

@mattbk
Copy link
Contributor Author

mattbk commented Jan 7, 2017

There are still some cardNumberInput calls in that function, so should probably see how important they are versus making autocomplete work. Autocomplete seems only to work if we don't mess with the number.

@mattbk
Copy link
Contributor Author

mattbk commented Jan 7, 2017

It is still checking whether the card is American Express or not, because it's telling me when to use 3 or 4 digits for CVV...

Also not validating numbers vs letters.

@nobodxbodon
Copy link
Contributor

May I ask on which instance you are testing? "review app" mentioned here?

@mattbk
Copy link
Contributor Author

mattbk commented Jan 7, 2017

That's a question for @whit537, I have no idea. I'm checking this locally.

@mattbk
Copy link
Contributor Author

mattbk commented Jan 7, 2017

@nobodxbodon, I honestly don't know what you're asking. 😄

@chadwhitacre
Copy link
Contributor

I think the answer is that you're "checking this locally"?

@nobodxbodon
Copy link
Contributor

@mattbk actually I was referring to your initial comment

testing locally is difficult because Chrome won’t autofill unless the connection is secure

So I wondered if you tested the commits on some staging/dev instance instead of locally.

@mattbk
Copy link
Contributor Author

mattbk commented Jan 7, 2017

So I wondered if you tested the commits on some staging/dev instance instead of locally.

I didn't know of anything like that already set up, so I played around with it until I realized that the message that autocomplete won't work actually follows the same behavior. So whether it will actually autocomplete the card doesn't matter, all that should matter is that the message comes up on the first number entered, rather than after n numbers and then a backspace.

I'm not convinced yet that the card number can't be passed through at least some of the subroutines in Gratipay.routes.cc.formatInputs(), in which case I need to dig deeper into that function and what it does.

@mattbk
Copy link
Contributor Author

mattbk commented Jan 8, 2017

From my experiments, I don't think we can play nice with autocomplete and validate the way we do now. However, https://github.com/braintree/card-validator looks like something we could use.

@whit537, does that seem reasonable, or is it hard to add something like this?

@chadwhitacre
Copy link
Contributor

Seems reasonable to me. 👍

@mattbk
Copy link
Contributor Author

mattbk commented Jan 14, 2017

Is anyone interested in taking this on? Running out of time in my day to give this the attention it deserves.

(@whit537, this doesn't include you jumping in and wasting another day. I'm trying to focus on only opening things I know I can finish.)

@mattbk
Copy link
Contributor Author

mattbk commented Apr 6, 2017

@EdOverflow is there any security reason for us to not support Chrome Autofill, aside from the user storing their credit card info in Chrome itself?

@EdOverflow
Copy link
Contributor

@mattbk I usually don't encourage people to use the Chrome Autofill, because of the following: https://github.com/anttiviljami/browser-autofill-phishing/blob/master/readme.md

That said, I don't think it would hurt to implement it on Gratipay. I will take a look when everything is done.

@mattbk
Copy link
Contributor Author

mattbk commented Apr 7, 2017

Rebased on master?

@chadwhitacre
Copy link
Contributor

Ish? Needs work, I think. Want me to try to clean it up?

@mattbk
Copy link
Contributor Author

mattbk commented Apr 7, 2017

Want me to try to clean it up?

If you have a chance. I wasn't going to go any further today, but apparently my drive-by rebasing needs work.

@chadwhitacre
Copy link
Contributor

I think you maybe did whatever @kaguillera sometimes does. :-)

Look okay now? (Was c88281f.)

@chadwhitacre
Copy link
Contributor

@mattbk Is this ready for review?

@mattbk
Copy link
Contributor Author

mattbk commented May 17, 2017

No, I never learned how to integrate https://github.com/braintree/card-validator. 😞

@mattbk
Copy link
Contributor Author

mattbk commented May 17, 2017

...and without that, the way to fix is to remove validation during card entry. IMHO this is fine...

@mattbk
Copy link
Contributor Author

mattbk commented Jun 21, 2017

I looked at this again. I think it's ready for review once green. Essentially it doesn't pass the card number through our internal validation, which lets Chrome begin filling in the card number immediately. It also adds all the right tags to the fields for Chrome autocomplete to work.

@mattbk
Copy link
Contributor Author

mattbk commented Jun 21, 2017

image

@mattbk mattbk closed this Jun 21, 2017
@mattbk mattbk deleted the chrome-autofill branch June 21, 2017 22:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants