keymanapp / keyman

Keyman cross platform input methods system running on Android, iOS, Linux, macOS, Windows and mobile and desktop web
https://keyman.com/
Other
372 stars 102 forks source link

change(web): now utilizes 'inputMode="none"' on supported touch devices πŸ“΄ #7343

Closed jahorton closed 1 year ago

jahorton commented 1 year ago

Addresses #3030.

This will remove our old "touch alias element" workaround for touch devices; said workaround predates widespread support inputMode attribute, hence why it's stuck around for so long. Note current compatibility charts: https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/inputmode#browser_compatibility

That said, we're not 100% sure which version of Safari is actually needed for proper compatibility: iOS 12.2 Safari doesn't interpret "none" in the manner we expect. Current iOS versions definitely do, though.

Note that this will cause a break in behavior for older devices than those indicated above. It's not the "best" thing, but we're doing this for a number of reasons - this allows the Keyman Engine for Web to handle input on touch-based devices much better than before.

Note that design-mode iframes (which are used for some forms of rich-text editing) are still off-limits due to some internal architectural issues for related event-handler setup. I imagine that it would be possible to address, but the changes there would be too significant to "fold in" to this PR. (Such changes would deserve their own focused PR for review.)


While this does drop a feature used by notably old browsers, based on the notes above, we've determined that continuing to maintain the feature would come at too high of an ongoing cost to the majority of users. While it might be possible to instead add some sort of auto-detection to selectively enable touch-aliasing when needed for older devices, adding that would further increase complexity and the maintenance load involved, especially for parts managing the element types listed above. Note the scale of the changeset - there's been quite a significant amount of code devoted to maintaining the status quo on this.

User Testing

This is a very notable change to KMW as used in web browsers, rather than in the mobile apps. As such, user testing will be focused on such environments.

Test Environments

Use the KeymanWeb Test Home link below for all user tests!

Test Specs

There's a strong argument to be made for more unit tests, though that may be best addressed via Web regression testing at the end of the chain.

Ideas for more:

Other notes:

Known bug:

This (disabled) test will fail because standard elements do not reset predictive context when the caret is relocated. In fact... there's a fair shot they may not properly be updating context at all. Either way, this matches current behavior on master.

keymanapp-test-bot[bot] commented 1 year ago

User Test Results

Test specification and instructions

Test Artifacts

bharanidharanj commented 1 year ago

GROUP_ANDROID: Use any Android 8.0+ mobile device (real or emulated)

..English Keyboard

..Khmer Keyboard

..Norwegian Keyboard

bharanidharanj commented 1 year ago

GROUP_IOS: Use any iOS 12.4+ mobile device (real or emulated)

..English Keyboard

..Khmer Keyboard

..Norwegian Keyboard

mcdurdin commented 1 year ago

Thanks @bharanidharanj for the tests!

GROUP_ANDROID - TEST_KEYBOARD_DISPLAY

  • However, after re-selecting the text box at the top ("Type here", the OSK shrink its from actual size on the page.

@jahorton I think this is something I've seen at times -- something about init order I think. Probably should be tracked in a separate issue?

mcdurdin commented 1 year ago

GROUP_IOS - TEST_KEYBOARD_DISPLAY However, after clicking the Add button, the system keyboard does not appear on the Screen.

GROUP_IOS - TEST_CONTENT_EDITABLE

  • However, there is an extra space appearing on the right side of the iPhone screen.

@jahorton should these issues fail these tests?

mcdurdin commented 1 year ago

Before reviewing, a couple of thoughts that I had:

mcdurdin commented 1 year ago

I don't anticipate issues with other platforms, but for safety I'd like to regression test also the following:

  1. KeymanWeb Embedded -- i.e. Keyman for Android, Keyman for iPhone and iPad
  2. Keyman on desktop platforms

I think we should split the new page element type support features, even if they come almost free, into two separate PRs. This helps us for future tracking, as well as reducing the scope of this change. (If contentEditable then sorta kinda works here, but not 100%, that's fine. The aim of this PR should be single-purpose: removing old code, not introducing new features.)

(Note: 95706dc is a disconnected commit?)

If you are happy to do the split into multiple PRs, I will do my review then.

jahorton commented 1 year ago

GROUP_IOS: Use any iOS 12.4+ mobile device (real or emulated)

[...]

  • TEST_STANDARD_IFRAME (FAILED): Tested this in "Tests the new Attachment/Enablement API functionality" test page, and the OSK partially appears on the Screen. I was not able to change the keyboard since it is partially appearing on the screen.

iOS Safari is so annoying about this: the nav bar at the bottom really, really likes to get in our way. Combine its display with auto-scroll, and that's probably the cause. You should be able to scroll the page slightly, at which point, the globe key should become accessible. If scrolling the page even slightly results in the OSK showing the globe key, I'm willing to call that a pass - it's a known Safari interaction issue, even if not necessarily documented with a formal issue yet.

jahorton commented 1 year ago

Before reviewing, a couple of thoughts that I had:

  • Is this supported on iOS 12.4? My earlier note said 13.0. In either case, we need to make sure we update the corresponding data online.

From the MDN link above:

image

  • This is kinda separate, but it would be good if keymanweb.com could fall back to earlier version of kmw for earlier versions of iOS (and ancient versions of Android)?

Sure, why not?

jahorton commented 1 year ago

@jahorton should these issues fail these tests?

GROUP_IOS - TEST_KEYBOARD_DISPLAY However, after clicking the Add button, the system keyboard does not appear on the Screen.

He noted using the Simulator. I didn't think about that angle completely; if he can use the "show keyboard" shortcut (Cmd+Shift+K) once or twice on his emulating computer's hardware keyboard and it displays then, this is not a bug. It's a... Simulator "feature" that I have to remember about when testing.

GROUP_IOS - TEST_CONTENT_EDITABLE

  • However, there is an extra space appearing on the right side of the iPhone screen.

I'd like to note that the entire page is being coerced left, not just the OSK. It's as if the page itself has its width restricted... and this naturally would constrain the width of the OSK as well. Note how the red attention-grabbing box extends through the whole page, not just beside the OSK. I think this detail is significant.

This didn't exactly occur for any of the other tests though, so I wonder what happened here to cause this? I'm leaning toward not a bug, but if this consistently reproduces, maybe it is... though I'd be extremely curious as to exactly how this behavior arose and affects the entire page like this.

mcdurdin commented 1 year ago

From the MDN link above:

The point is, that MDN link is wrong. From #3030:

  • Safari on iOS - supported since iOS 12.2 (but not none).
jahorton commented 1 year ago

@bharanidharanj Sorry about this, but I did neglect an important note for proper iOS testing:

  • Important note: if using emulated iOS, make sure that Simulator is set to display the device's keyboard.
    • Hardware keyboard shortcut: ⇧ + ⌘ + K. Use that once or twice within a standard iOS app (i.e., not Keyman or a KMW page) until the keyboard shows, then perform the tests.

It's important that this be enabled for the tests, as whether or not the device's keyboard displays is a crucial aspect of the user tests above.

@keymanapp-test-bot retest GROUP_IOS TEST_KEYBOARD_DISPLAY TEST_CONTENT_EDITABLE TEST_STANDARD_IFRAME TEST_INPUT_FORMATTING

jahorton commented 1 year ago

From the MDN link above:

The point is, that MDN link is wrong. From #3030:

  • Safari on iOS - supported since iOS 12.2 (but not none).

Ah, I see that mentioned within this link mentioned there, now that I dig a little deeper:

https://css-tricks.com/everything-you-ever-wanted-to-know-about-inputmode/

jahorton commented 1 year ago

For what it's worth, I can tell that it works with "none" as expected on my local iPhone. In fact, it feels really, really smooth when swapping from the system keyboard to the KMW one, at least on the test page I tried it with. Not as much in reverse, since I didn't exactly aim to optimize the transition at any point.

mcdurdin commented 1 year ago

Well, we can dig really deep into Safari on iOS support:

Perhaps worth doing more digging at some point? The link above notes:

Don't show the keyboard if inputmode is none and a hardware keyboard is not attached.

Reload input views if the inputmode is none to ensure that if a hardware keyboard is attached while > editing an element with inputmode="none", we'll show the input accessory view once again.

So there's an area to test...

mcdurdin commented 1 year ago

Also: https://github.com/mdn/browser-compat-data/issues/17919

bharanidharanj commented 1 year ago

GROUP_IOS: Use any iOS 12.4+ mobile device (real or emulated)

jahorton commented 1 year ago

GROUP_IOS: Use any iOS 12.4+ mobile device (real or emulated)

* **TEST_KEYBOARD_DISPLAY (FAILED):** Before I started the test, I pressed ⇧ + ⌘ + K in the iPhone7 simulator and followed the steps as it mentioned in the Keyboard Display Test. After clicking the Add button in the 'Test Unminified Keymanweb' test page, the system keyboard still appears above the Keyman Keyboard.
   <img alt="" width="300" src="https://user-images.githubusercontent.com/19683143/193248885-6098673f-e047-4a9f-bcce-3df7c02c02bc.png">

* **TEST_CONTENT_EDITABLE (FAILED):** Here, I noticed that two Keyboards are appearing after clicking the Edit me! text. Also, long pressing the globe key shows only one keyboard (EuroLatin (SIL) in the Keyman menu.
   <img alt="" width="300" src="https://user-images.githubusercontent.com/19683143/193248899-d38a2ca0-b35b-4ce4-b8e9-5b91d0dfc7e3.png">

* **TEST_STANDARD_IFRAME (FAILED):** Not able to swap to Lao keyboard since it is showing only EuroLatin (SIL) keyboard after long pressing the globe key.
   <img alt="" width="300" src="https://user-images.githubusercontent.com/19683143/193248920-01c248fe-3bab-4e0b-8145-4a289992d233.png">

* **TEST_INPUT_FORMATTING (FAILED):** Not able to swap to Lao keyboard since it is showing only EuroLatin (SIL) keyboard after long pressing the globe key.
   <img alt="" width="300" src="https://user-images.githubusercontent.com/19683143/193248958-29b39c16-a7d5-40d0-a517-e519a542631d.png">

Which version of iOS simulator did you use for this test, @bharanidharanj? We've noticed that we don't actually know the minimum version this should work for, so if it's 12.4... well, that's useful info, at least.

jahorton commented 1 year ago

~I've found some errors after detaching page elements on touch, so that needs addressing before merge.~

Later edit: and, resolved.

jahorton commented 1 year ago

Found a couple more bits to remove and fixed an issue with touch-based text selection within attached inputs causing the OSK to hide.

jahorton commented 1 year ago

GROUP_IOS: Use any iOS 12.4+ mobile device (real or emulated)

* **TEST_KEYBOARD_DISPLAY (FAILED):** Before I started the test, I pressed ⇧ + ⌘ + K in the iPhone7 simulator and followed the steps as it mentioned in the Keyboard Display Test. After clicking the Add button in the 'Test Unminified Keymanweb' test page, the system keyboard still appears above the Keyman Keyboard.
   <img alt="" width="300" src="https://user-images.githubusercontent.com/19683143/193248885-6098673f-e047-4a9f-bcce-3df7c02c02bc.png">

* **TEST_CONTENT_EDITABLE (FAILED):** Here, I noticed that two Keyboards are appearing after clicking the Edit me! text. Also, long pressing the globe key shows only one keyboard (EuroLatin (SIL) in the Keyman menu.
   <img alt="" width="300" src="https://user-images.githubusercontent.com/19683143/193248899-d38a2ca0-b35b-4ce4-b8e9-5b91d0dfc7e3.png">

* **TEST_STANDARD_IFRAME (FAILED):** Not able to swap to Lao keyboard since it is showing only EuroLatin (SIL) keyboard after long pressing the globe key.
   <img alt="" width="300" src="https://user-images.githubusercontent.com/19683143/193248920-01c248fe-3bab-4e0b-8145-4a289992d233.png">

* **TEST_INPUT_FORMATTING (FAILED):** Not able to swap to Lao keyboard since it is showing only EuroLatin (SIL) keyboard after long pressing the globe key.
   <img alt="" width="300" src="https://user-images.githubusercontent.com/19683143/193248958-29b39c16-a7d5-40d0-a517-e519a542631d.png">

Which version of iOS simulator did you use for this test, @bharanidharanj? We've noticed that we don't actually know the minimum version this should work for, so if it's 12.4... well, that's useful info, at least.

Still waiting on a response to this, @bharanidharanj.

jahorton commented 1 year ago

Finally bothered trying to check the Safari version requirement via the Xcode's iOS Simulator.

In iOS 12.4: πŸŸ₯

image (1)

In iOS 13.2: βœ…

image (2)

Here, there's even a period where neither is shown (as the KMW OSK loads). It's clearly an "either-or" situation, unlike with 12.4.

I'd need to go downloading extra runtimes to provide a more precise investigation, but I get the feeling this means that the breaking point is iOS 13.


Also, I'm not sure if it's a Simulator-only thing, but "connecting" a hardware keyboard instantly disables the "software" (visual) keyboard on iOS. This prevented the odd Android-Chrome scenario mentioned in one of the comment threads - the issue where typing a physical keystroke force-displayed the OSK, regardless of inputMode setting.

jahorton commented 1 year ago

One download later... suspicion confirmed:

image (4)
jahorton commented 1 year ago

Now that we have a better idea what the minimum iOS version needed is...

@keymanapp-test-bot retest GROUP_IOS TEST_KEYBOARD_DISPLAY TEST_CONTENT_EDITABLE TEST_STANDARD_IFRAME TEST_INPUT_FORMATTING

bharanidharanj commented 1 year ago

GROUP_IOS: Use any iOS 13.0+ mobile device (real or emulated)

..English

..Khmer and Norwegian

bharanidharanj commented 1 year ago

GROUP_ANDROID: Use any Android 8.0+ mobile device (real or emulated)

keyman-server commented 1 year ago

Changes in this pull request will be available for download in Keyman version 16.0.82-alpha