Page MenuHomePhabricator

Fix username typeahead in Remarkup with German keyboard layout
ClosedPublic

Authored by chad on Jul 24 2017, 12:26 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 19, 11:34 PM
Unknown Object (File)
Wed, Dec 18, 7:04 PM
Unknown Object (File)
Tue, Dec 17, 4:14 AM
Unknown Object (File)
Mon, Dec 16, 6:45 PM
Unknown Object (File)
Sat, Dec 7, 4:48 PM
Unknown Object (File)
Fri, Dec 6, 5:53 AM
Unknown Object (File)
Tue, Dec 3, 12:47 PM
Unknown Object (File)
Thu, Nov 28, 10:58 AM
Subscribers
Tokens
"Party Time" token, awarded by OCram.

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
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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 edited reviewers, added: benwick; removed: chad.
chad edited reviewers, added: epriestley; removed: Blessed Reviewers.

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

Here's, e.g., a user complaining that another piece of software implemented a similar behavior, hijacking +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.