search

mozilla / fxa

Monorepo for Mozilla Accounts (formerly Firefox Accounts)
https://mozilla.github.io/ecosystem-platform/
Mozilla Public License 2.0
590 stars 212 forks source link

Sign in confirmation screen shows the location as "Kanada" instead of "Canada" #5426

Closed data-sync-user closed 3 years ago

data-sync-user commented 4 years ago

Finished signing in on iOS using the QR pairing flow on beta. On the confirmation page on desktop Canada is spelled as Kanada.

!image (9).png!

┆Issue is synchronized with this Jira Bug ┆Attachments: image (9).png | IMG_8936.jpeg | Kanada.png | Screen Shot 2020-05-22 at 11.02.46 AM.png ┆Issue Number: FXA-1965

data-sync-user commented 4 years ago

➤ Wil Clouser commented:

what

data-sync-user commented 4 years ago

➤ Jody Heavener commented:

i don't see the problem here

data-sync-user commented 4 years ago

➤ Wil Clouser commented:

[~jmorrison@mozilla.com] did the maxmind library change recently?  It seems like we would have noticed this before now

[~jheavener@mozilla.com] or [~adavis@mozilla.com] (or anyone else in Canada?) can you reproduce this?

data-sync-user commented 4 years ago

➤ Ana Medinac commented:

Steps to reproduce:

  1. "Sign in to Sync" on iOS Beta v26

  2. click "Ready to Scan" on iOS

  3. Go to https://accounts.firefox.com/pair/ ( https://accounts.firefox.com/pair/ ) on desktop (when already signed in to FxA) then click Show Code

  4. Scan

The desktop UI to confirm the code will have Canada spelled as "Kanada".

data-sync-user commented 4 years ago

➤ Alex Davis commented:

I was about to check but my Android device needed to be recharged and now it's gone on an eternal path of OS and app updates. I might get back to you tomorrow. 😬

data-sync-user commented 4 years ago

➤ Alex Davis commented:

  1. "Sign in to Sync" on iOS Beta v26

Oh great, let me check iOS. I was looking at pairing on Android.

data-sync-user commented 4 years ago

➤ Alex Davis commented:

CONFIRMED

It's really weird. I see "Noth Vancouver, Canada" on my phone but "North Vancouver, Kanada" on desktop. (each device gives the location)

data-sync-user commented 4 years ago

➤ Jody Heavener commented:

That is very strange.

data-sync-user commented 4 years ago

➤ Alex Davis commented:

I just retested today and I still see it AND I confirm that both devices had the exact same IP despite displaying the country names differently. I attached screenshots.

data-sync-user commented 4 years ago

➤ Wil Clouser commented:

[~adavis@mozilla.com] - would you dump the Accept-Lang headers that each device is sending?

data-sync-user commented 4 years ago

➤ Benjamin Bangert commented:

I'm getting this too, device shows 'California' correctly, desktop shows 'Kalifornien'. Desktop Accept-Language is: en-US,en;q=0.5

vasilicatamas commented 4 years ago

I'm also reproducing this issue on Desktop when pairing with an iOS device. On Desktop it is displayed Rumänien while on iOS device appears written Romania. This seems like all the countries are translated in German language no matter what Desktop locals are used (tested with en-US, fr, es-ES).

data-sync-user commented 4 years ago

➤ Wil Clouser commented:

[~adavis@mozilla.com] will check another email

[~vbudhram@mozilla.com] will check an idea he has

vladikoff commented 4 years ago

The default locale is en in https://github.com/mozilla/fxa/blob/master/packages/fxa-geodb/lib/defaults.js#L13

Maybe something changed in the maxmind db and it does not accept that locale anymore?

data-sync-user commented 4 years ago

➤ Alex Davis commented:

Login confirmation email looks good. Says Canada.

data-sync-user commented 4 years ago

➤ Wil Clouser commented:

pinging [~jmorrison@mozilla.com]  again.  Did something in the maxmind db change around mid May?  Did we upgrade to a newer version or anything? /cc [~jbuckley@mozilla.com]

data-sync-user commented 4 years ago

➤ Jon Buckley commented:

We upgrade the version of the Maxmind DB weekly

data-sync-user commented 4 years ago

➤ Vijay Budhram commented:

A quick audit of the code, it appears that the page gets this location data from the desktop browser during the pairing handshake. It doesn't use any location data from any FxA servers.

[~vfilippov@mozilla.com] do you know what might be happening on the desktop side of things here?

rfk commented 4 years ago

I believe the geolocation data here ultimately comes from channelserver, which says this in its readme:

This will attempt to localize the geolocation data based on the preferred Accept-Languages: HTTP header.
If no header is provided, results are unspecified (although probably in German).

Which...honestly I would read the "probably in German" part as a joke, but maybe it's not!

rfk commented 4 years ago

/cc @jrconlin for visibility re: channelserver above ^

jrconlin commented 4 years ago

Sorry, not sure why I didn't see this until now. Sadly, German is not a joke. It's the first language in alphabetical order for the geo lookup database (Deutsch). As Vijay noted, channel server reads and parses the Accept-Languages header and attempt to return the most preferred language. Not sure why it's failing since I'm pretty sure I have a test that checks for a string remarkably similar to what's being passed in.

Feel free to boot this bug over to channel server and I can try looking into it.

clouserw commented 4 years ago

Filed for channelserver at https://github.com/mozilla-services/channelserver/issues/83

rfk commented 4 years ago

It's also possible that we're not correctly sending the accept-language header in the connection to channelserver in some cases.

jrconlin commented 4 years ago

Yeah, working with jbuck to see if that might be the case. I checked the code and it should? be working fine, but we default to "none" explicitly if Accept-Languages is not passed. I'd be curious why it's working for mobile, though.

rfk commented 4 years ago

Just to check my assumptions here, is it true that we're only seeing this behaviour on iOS?

If so, I wonder if this is an issue with how iOS sends the accept-language header in websocket connections vs normal HTTP connections.

rfk commented 4 years ago

To add a few more words to the above, here's my mental model of what might be happening:

  1. The iOS device scans the pairing code QR, extracts the URL therein, and opens it in a special webview.
  2. The pairing supplicant code on accounts.firefox.com loads up in that webview and opens a websocket to channelserver
  3. The channelserver extracts the geolocation and accept-language data from the websocket connection and makes it available to the desktop device for display

So if the connection at (2) does not include the accept-language header then the channelserver might default to showing the geolocation data in German, producing the confirmation page shown here. There have definitely been bugs with websocket connections not including accept-language in the past.

Unfortunately I'm not well set up to debug this on an iOS device, but if someone has an iOS device and is able to debug the outgoing connections made at step (2) above, I think it would be a good place to look.

data-sync-user commented 4 years ago

➤ Benjamin Bangert commented:

I saw this come up incorrectly on the desktop. The device being paired showed it correctly.

rfk commented 4 years ago

I saw this come up incorrectly on the desktop.

Right, the desktop device displays the info about the other device, info which ultimately comes from channelserver from the websocket handshake. Was the other device iOS in this case?

data-sync-user commented 4 years ago

➤ Benjamin Bangert commented:

Yep, the other device was iOS.

garvankeeley commented 4 years ago

I don't see a screenshot in this bug so adding one:

image
jrconlin commented 4 years ago

Ah, so I was confused, desktop may not be impacted. If this is happening mostly on iOS, could it be that iOS is not providing a proper Accept-Language header?

@jbuck it might be possible to sniff for an iOS Request and check the header in the logs.

jbuck commented 4 years ago

@jrconlin you are correct, Fx iOS is sending an Accept-Language header of -. I'm not sure how that converts to de though :)

garvankeeley commented 4 years ago

Looks like a WebKit bug, it is supposed to be setting accept-language correctly. We may be able to hack around this on iOS, but HTTP headers are not modifiable on anything except an initial load request (i.e. if we call wkwebview.load(theRequestWithModifiedHeader), any sub-resource or consecutive load after that is not exposed to the client.

This answer is accurate: https://stackoverflow.com/questions/36868935/how-to-set-custom-http-headers-to-requests-made-by-a-wkwebview

jrconlin commented 4 years ago

@jbuck heh, the "de" bit comes in because that's the first language that the geolookup library has handy. Since nothing is specified, it just goes with the first one. I suppose we could modify the default to use English instead, but not sure that's really a better solution.

rfk commented 4 years ago

I suppose we could modify the default to use English instead, but not sure that's really a better solution.

I agree in principle; OTOH the whole of FxA will default to English if you don't provide an accept-lang header, so defaulting to english here as well would at least be consistently the wrong thing rather than a different wrong thing.

I also fear the bug may be specific to the websocket connection made from within the wkwebview, rather than a property of the wkwebview as a whole...since I feel like we would have gotten bug reports by now if the whole FxA experience on iOS was not localized correctly. But that may also mean that we can hack around it in web content inside FxA.

rfk commented 4 years ago

But that may also mean that we can hack around it in web content inside FxA.

For example, we could have FxA web content append the accept-lang header as a query param on the websocket URL, and have channelserver grab it from there if present; awful, but at least it's awful for us instead of for the user.

clouserw commented 3 years ago

Thanks to @jrconlin for the fix in https://github.com/mozilla-services/channelserver/issues/83 . I'm closing this.

rfk commented 3 years ago

IIUC German users will now incorrectly see Canada instead of Kanada on this screen when connecting an iOS device; should we spin off a separate bug for that issue?