Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
data-sync-user opened this issue May 22, 2020 · 38 comments
Assignees

Comments

@data-sync-user
Copy link
Collaborator

data-sync-user commented May 22, 2020

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
Copy link
Collaborator Author

➤ Wil Clouser commented:

what

@data-sync-user
Copy link
Collaborator Author

➤ Jody Heavener commented:

i don't see the problem here

@data-sync-user data-sync-user changed the title Sign in confirmation displays Canada as Kanada on desktop Beta Sign in confirmation screen shows the location as "Kanada" instead of "Canada" May 22, 2020
@data-sync-user
Copy link
Collaborator Author

➤ 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
Copy link
Collaborator Author

➤ 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
Copy link
Collaborator Author

➤ 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
Copy link
Collaborator Author

➤ 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
Copy link
Collaborator Author

➤ 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
Copy link
Collaborator Author

➤ Jody Heavener commented:

That is very strange.

@data-sync-user
Copy link
Collaborator Author

➤ 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
Copy link
Collaborator Author

➤ Wil Clouser commented:

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

@data-sync-user
Copy link
Collaborator Author

➤ 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
Copy link

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
Copy link
Collaborator Author

➤ Wil Clouser commented:

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

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

@vladikoff
Copy link
Contributor

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
Copy link
Collaborator Author

➤ Alex Davis commented:

Login confirmation email looks good. Says Canada.

@data-sync-user
Copy link
Collaborator Author

➤ 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
Copy link
Collaborator Author

➤ Jon Buckley commented:

We upgrade the version of the Maxmind DB weekly

@data-sync-user
Copy link
Collaborator Author

➤ 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
Copy link
Contributor

rfk commented Jul 7, 2020

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
Copy link
Contributor

rfk commented Jul 7, 2020

/cc @jrconlin for visibility re: channelserver above ^

@jrconlin
Copy link
Contributor

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
Copy link
Member

Filed for channelserver at mozilla-services/channelserver#83

@rfk
Copy link
Contributor

rfk commented Jul 14, 2020

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

@jrconlin
Copy link
Contributor

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
Copy link
Contributor

rfk commented Jul 14, 2020

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
Copy link
Contributor

rfk commented Jul 14, 2020

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
Copy link
Collaborator Author

➤ Benjamin Bangert commented:

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

@rfk
Copy link
Contributor

rfk commented Jul 15, 2020

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
Copy link
Collaborator Author

➤ Benjamin Bangert commented:

Yep, the other device was iOS.

@garvankeeley
Copy link

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

@jrconlin
Copy link
Contributor

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.

@data-sync-user data-sync-user changed the title Sign in confirmation screen shows the location as "Kanada" instead of "Canada" Sign in confirmation screen shows the location as "Kanada" instead of "Canada" Aug 7, 2020
@jbuck
Copy link
Member

jbuck commented Sep 15, 2020

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

@garvankeeley
Copy link

garvankeeley commented Sep 15, 2020

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
Copy link
Contributor

@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
Copy link
Contributor

rfk commented Sep 15, 2020

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
Copy link
Contributor

rfk commented Sep 15, 2020

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
Copy link
Member

Thanks to @jrconlin for the fix in mozilla-services/channelserver#83 . I'm closing this.

@rfk
Copy link
Contributor

rfk commented Sep 23, 2020

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants