Page MenuHomePhabricator

Fix username typeahead in Remarkup with German keyboard layout
ClosedPublic

Authored by chad on Jul 24 2017, 12:26 PM.

Details

Summary

Ref T10252. The previous fix rPa8a9fddb0738 only works for macOS.
Under Windows the @ symbol is composed of AltGr+q. For Chrome and Edge the "AltGr" keypressEvent is like pressing the Control key and the Alt key at the same time.
This fix changes the condition in such a way, that this case (pressing Control and Alt at the same time) is not blocked.

Test Plan

Testing for the issue:

  • Launch Windows 10, Select German Keyboard, Use latest Chrome (60)
  • Observe typing @ does not trigger typeahead
  • Apply patch, retest, see typeahead.

Regression tested:

  • Windows 10, Chrome, Firefox, Edge
  • Mac OS, Chrome, Firefox, Safari
  • Keyboard layouts, English, French, German, Spanish

All tests passed

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

benwick created this revision.Jul 24 2017, 12:26 PM
OCram awarded a token.Jul 24 2017, 1:02 PM
chad edited edge metadata.Jul 24 2017, 1:35 PM

Let me try to scare up some decent way to test this today then I can just let it fly and revert if we get more tasks about it.

chad commandeered this revision.Aug 3 2017, 4:30 PM
chad edited reviewers, added: benwick; removed: chad.
chad updated this revision to Diff 44065.Aug 3 2017, 4:31 PM
  • add celerity map
chad edited the test plan for this revision. (Show Details)Aug 3 2017, 5:12 PM
chad edited reviewers, added: epriestley; removed: Blessed Reviewers.
chad added a comment.Aug 3 2017, 5:15 PM

@epriestley I am comfortable enough to ship this - at least I tested it reasonably well.

epriestley accepted this revision.Aug 3 2017, 5:44 PM

Here's, e.g., a user complaining that another piece of software implemented a similar behavior, hijacking Control+Alt+V which they intend to mean "Paste Special":

https://superuser.com/questions/403011/evernote-hijacked-my-ctrlaltv-shortcut-how-can-i-get-it-back

However, we don't hijack the keystroke and don't autocomplete on "V", so I think this is fine until we run into tangible issues with it.

This revision is now accepted and ready to land.Aug 3 2017, 5:44 PM

(I also think "Paste Special" probably isn't meaningful in browsers, I think that user was maybe using a Windows extension or toolbar or something, not complaining about website/browser behavior.)

This revision was automatically updated to reflect the committed changes.