Page MenuHomePhabricator
Create Task
Maniphest T136988

QR code displayed inconsistently
Closed, ResolvedPublic
Actions

Description

When enabling two-factor authentication, the QR code which is scanned for enrollment occasionally does not render. There appears to be in issue with the client-side code used to draw the code. This is confusing for users who are new to the use of Google Authenticator and other such tools as part of their authentication flow.

Here are reproduction steps that seem to work fairly consistently. They require that you not already have two-factor enabled for the account used for testing.

  1. Clear your browser cache.
  2. Login to https://en.wikipedia.org
  3. Click "Preferences"
  4. Click "Enable two-factor authentication"
  5. Observe that the QR code is displayed
  6. Click "Preferences"
  7. Click "Enable two-factor authentication"
  8. Observe that the QR code is not displayed
  9. Shift-Refresh the page
  10. Observe that the QR code is displayed
  11. Click "Preferences"
  12. Click "Enable two-factor authentication"
  13. Observe that the QR code is not displayed
  14. Click "Preferences"
  15. Click "Enable two-factor authentication"
  16. Observe that the QR code is not displayed
  17. Click "Preferences"
  18. Click "Enable two-factor authentication"
  19. Observe that the QR code is not displayed

So, my hypothesis is that this is a timing issue and has to do with loading from browser cache. It seems that the QR code is not rendered when the JavaScript need is available in local cache. Maybe. Sometimes it does render when loading from local cache. so I'm not certain.

Details

SubjectRepoBranchLines +/-
Fixup qrcode-generating js, to stop race condition.mediawiki/extensions/OATHAuthwmf/1.28.0-wmf.6+30 -20
Fixup qrcode-generating js, to stop race condition.mediawiki/extensions/OATHAuthwmf/1.28.0-wmf.7+30 -20
Fixup qrcode-generating js, to stop race condition.mediawiki/extensions/OATHAuthmaster+30 -20
Customize query in gerrit

Event Timeline

• dpatrick created this task.Jun 3 2016, 9:37 PM
Restricted Application added subscribers: Zppix, Aklapper. · View Herald TranscriptJun 3 2016, 9:37 PM
• dpatrick triaged this task as Unbreak Now! priority.Jun 3 2016, 9:38 PM
Restricted Application added subscribers: Luke081515, TerraCodes, Urbanecm. · View Herald TranscriptJun 3 2016, 9:38 PM
Parent5446 subscribed.Jun 3 2016, 9:44 PM

Is there a scenario in which this can be reproduced? Or is it seemingly random?

Parent5446 moved this task from Backlog to Need for Deployment on the MediaWiki-extensions-OATHAuth board.Jun 3 2016, 9:46 PM
• dpatrick added a comment.Jun 3 2016, 10:07 PM

It seems to be random, and I've experienced it in both Firefox and Chrome.

• dpatrick added a subscriber: Jalexander.Jun 3 2016, 10:11 PM

@Jalexander mentioned yesterday that he encountered this during enrollment as well, but he knew to enter the key manually. Many users will not (which is an issue to be emphasized in documentation resulting from a separate phab ticket).

• dpatrick moved this task from Need for Deployment to User Experience on the MediaWiki-extensions-OATHAuth board.Jun 8 2016, 5:39 PM
Restricted Application added a subscriber: Poyekhali. · View Herald TranscriptJun 8 2016, 5:39 PM
• dpatrick renamed this task from QR code displayed inconsistenly to QR code displayed inconsistently.Jun 9 2016, 1:18 AM
• dpatrick added a comment.Jun 9 2016, 7:10 PM

Could this be caused by the JavaScript creating the QRCode running before the DOM is fully loaded?

• dpatrick added a subscriber: Krinkle.Jun 9 2016, 7:11 PM

@Krinkle, do you have any ideas about this?

• dpatrick updated the task description. (Show Details)Jun 10 2016, 12:28 AM
Bawolff claimed this task.Jun 14 2016, 8:37 PM
Tgr subscribed.Jun 16 2016, 11:39 AM

Can't reproduce in Chrome nor in Firefox. Code looks sane too - the DOM placeholder is generated by serverside code, the QR image generator is loaded via ResourceLoader::makeInlineScript which places the script into the RL queue which is processed on DOM ready.

Bawolff added a comment.Jun 20 2016, 7:09 AM
In T136988#2384996, @Tgr wrote:

Can't reproduce in Chrome nor in Firefox. Code looks sane too - the DOM placeholder is generated by serverside code, the QR image generator is loaded via ResourceLoader::makeInlineScript which places the script into the RL queue which is processed on DOM ready.

RL queue is not necessarily processed on DOM ready. It only garuntees that RL is ready. In fact, I suspect what is happening is that on requests where the startup module is locally cached, RL is ready before <div id="qrcode"> is parsed by the browser, so $( 'MediaWiki-extensions-UrlShortener' ) at the time of call returns 0 results, resulting in nothing happening.

I wonder if the reason this is impossible to reproduce locally, is that from localhost, there is no network delay and a much larger MTU (and due to output buffering, the page comes all at once), so the page wouldn't be split over multiple packets. Hence there is no time between the browser getting the instruction to load the startup module and the rest of the page.

gerritbot subscribed.Jun 20 2016, 7:36 AM

Change 295191 had a related patch set uploaded (by Brian Wolff):
Fixup qrcode-generating js, to stop race condition.

https://gerrit.wikimedia.org/r/295191

gerritbot added a project: Patch-For-Review.Jun 20 2016, 7:36 AM
gerritbot added a comment.Jun 21 2016, 7:39 PM

Change 295191 merged by jenkins-bot:
Fixup qrcode-generating js, to stop race condition.

https://gerrit.wikimedia.org/r/295191

• dpatrick added a comment.Jun 21 2016, 7:51 PM

@Bawolff Thanks a lot Brian. This is schedule for deployment in the early SWAT on June 22nd.

ReleaseTaggerBot added a project: MW-1.28-release (WMF-deploy-2016-06-28_(1.28.0-wmf.8)).Jun 22 2016, 5:00 AM
gerritbot added a comment.Jun 22 2016, 3:06 PM

Change 295510 had a related patch set uploaded (by Thcipriani):
Fixup qrcode-generating js, to stop race condition.

https://gerrit.wikimedia.org/r/295510

gerritbot added a comment.Jun 22 2016, 3:07 PM

Change 295511 had a related patch set uploaded (by Thcipriani):
Fixup qrcode-generating js, to stop race condition.

https://gerrit.wikimedia.org/r/295511

gerritbot added a comment.Jun 22 2016, 3:11 PM

Change 295511 merged by jenkins-bot:
Fixup qrcode-generating js, to stop race condition.

https://gerrit.wikimedia.org/r/295511

gerritbot added a comment.Jun 22 2016, 3:11 PM

Change 295510 merged by jenkins-bot:
Fixup qrcode-generating js, to stop race condition.

https://gerrit.wikimedia.org/r/295510

• dpatrick added a comment.Jun 22 2016, 3:35 PM

This has been deployed and appears to be working well!

ReleaseTaggerBot added projects: MW-1.28-release (WMF-deploy-2016-06-14_(1.28.0-wmf.6)), MW-1.28-release (WMF-deploy-2016-06-21_(1.28.0-wmf.7)).Jun 22 2016, 4:00 PM
Bawolff closed this task as Resolved.Jun 23 2016, 6:53 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits