[ iOS ] 3x editing/spelling/* (layout-tests) are constant failures #27003

whsieh · 2024-04-09T01:09:56Z

`697b1a8`

[ iOS ] 3x editing/spelling/* (layout-tests) are constant failures
https://bugs.webkit.org/show_bug.cgi?id=271864
rdar://125585822

Reviewed by Aditya Keerthi.

Among the many other changes to adopt `BETextInput`, iOS 17.4 refactored `WKContentView` to stop
relying on the UIKit internal method `-_requiresKeyboardWhenFirstResponder`. Instead, UIKit now
returns `YES` when the delegate is a `BETextInput`, as long as either of the following are true:

1. The hardware keyboard is connected.
2. An editable element is focused.

However, a consequence of this refactoring is that `WKContentView` now vends text input traits to
keyboard code in UIKit when a hardware keyboard is attached, even in the case where there is no
focused element. In practice, this doesn't really matter, since these text input traits (which
contain `UITextAutocorrectionTypeNo`) are effectively unused. However, in the case of these three
layout tests that programmatically focus text inputs, insert text, and expect spellchecking to
occur outside of the context of any input session, we end up failing because the spelling
corrections are disabled by the traits.

To fix this, restore pre-iOS 17.4 behavior by reverting to `WKExtendedTextInputTraits`'s default
values when no element is focused (or being focused). Namely, this ensures that the autocorrection
type is set to `UITextAutocorrectionTypeDefault`, which matches shipping behavior in the case when
the user isn't focusing anything editable, but still enables continuous spellchecking when
programmatically inserting text.

* LayoutTests/platform/ios/TestExpectations:

Mark these layout test as passing again.

* Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView _updateTextInputTraits:]):
(-[WKContentView extendedTraitsDelegate]):

See above for more details.

* Source/WebKit/UIProcess/ios/WKExtendedTextInputTraits.h:
* Source/WebKit/UIProcess/ios/WKExtendedTextInputTraits.mm:
(-[WKExtendedTextInputTraits init]):
(-[WKExtendedTextInputTraits setSelectionColorsToMatchTintColor:]):
(-[WKExtendedTextInputTraits restoreDefaultValues]):

Add a new method to restore all default text input traits.

(-[WKExtendedTextInputTraits setSelectionBarColor:]): Deleted.
(-[WKExtendedTextInputTraits selectionBarColor]): Deleted.

Remove some old versions of the `UIAsyncTextInput` implementation that are no longer necessary,
since they've all been superceded by `BETextInput`.

Canonical link: https://commits.webkit.org/277233@main

41e070e

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-skia
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 tv	🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv-sim		✅ 🧪 api-gtk
✅ 🛠 🧪 merge	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-04-09T01:10:02Z

EWS run on previous version of this PR (hash 9254368)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	⏳ 🧪 wpe-wk2
✅ 🧪 webkitperl	⏳ 🧪 ios-wk2	~~🧪 api-mac~~	~~🧪 api-wpe~~
	⏳ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-skia
	⏳ 🧪 api-ios	~~🧪 mac-wk2~~	✅ 🛠 gtk
	~~🛠 tv~~	⏳ 🧪 mac-AS-debug-wk2	~~🧪 gtk-wk2~~
	❌ 🛠 tv-sim		~~🧪 api-gtk~~
	~~🛠 watch~~
	~~🛠 watch-sim~~

webkit-early-warning-system · 2024-04-09T01:26:44Z

EWS run on current version of this PR (hash 41e070e)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-skia
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 tv	🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv-sim		✅ 🧪 api-gtk
✅ 🛠 🧪 merge	✅ 🛠 watch
	✅ 🛠 watch-sim

pxlcoder · 2024-04-09T01:31:31Z

Source/WebKit/UIProcess/ios/WKExtendedTextInputTraits.mm

    self.selectionHighlightColor = shouldUseTintColor ? [tintColor colorWithAlphaComponent:selectionHighlightAlphaComponent] : nil;
 }

+- (void)restoreDefaultValues


Is there any risk if the property list in this method gets out-of-date? (I think no, but want to confirm)

Right — it would get out of date if we added a new @property to WKExtendedTextInputTraits, but didn't update this method as well. That said, I think it's not that easy to make that mistake, as long as whoever's adding the new property searches the existing code for any of the other existing properties, and follows the same precedent.

I guess my question is more specifically: "What would the effect be (if any) if the list were to get out-of-date?".

Ah, I see. It's pretty unlikely this would user-facing effects even if the list were to get out-of-date, since this would only affect the case where UIKit relies on text input traits outside of text input. The only other trait that falls into this category is selection highlight and handle color (which I made sure to update separately, in this patch), and apparently this autocorrectionType which influences whether or not continuous spellchecking is enabled.

whsieh · 2024-04-09T05:20:30Z

Thanks for the review!

https://bugs.webkit.org/show_bug.cgi?id=271864 rdar://125585822 Reviewed by Aditya Keerthi. Among the many other changes to adopt `BETextInput`, iOS 17.4 refactored `WKContentView` to stop relying on the UIKit internal method `-_requiresKeyboardWhenFirstResponder`. Instead, UIKit now returns `YES` when the delegate is a `BETextInput`, as long as either of the following are true: 1. The hardware keyboard is connected. 2. An editable element is focused. However, a consequence of this refactoring is that `WKContentView` now vends text input traits to keyboard code in UIKit when a hardware keyboard is attached, even in the case where there is no focused element. In practice, this doesn't really matter, since these text input traits (which contain `UITextAutocorrectionTypeNo`) are effectively unused. However, in the case of these three layout tests that programmatically focus text inputs, insert text, and expect spellchecking to occur outside of the context of any input session, we end up failing because the spelling corrections are disabled by the traits. To fix this, restore pre-iOS 17.4 behavior by reverting to `WKExtendedTextInputTraits`'s default values when no element is focused (or being focused). Namely, this ensures that the autocorrection type is set to `UITextAutocorrectionTypeDefault`, which matches shipping behavior in the case when the user isn't focusing anything editable, but still enables continuous spellchecking when programmatically inserting text. * LayoutTests/platform/ios/TestExpectations: Mark these layout test as passing again. * Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm: (-[WKContentView _updateTextInputTraits:]): (-[WKContentView extendedTraitsDelegate]): See above for more details. * Source/WebKit/UIProcess/ios/WKExtendedTextInputTraits.h: * Source/WebKit/UIProcess/ios/WKExtendedTextInputTraits.mm: (-[WKExtendedTextInputTraits init]): (-[WKExtendedTextInputTraits setSelectionColorsToMatchTintColor:]): (-[WKExtendedTextInputTraits restoreDefaultValues]): Add a new method to restore all default text input traits. (-[WKExtendedTextInputTraits setSelectionBarColor:]): Deleted. (-[WKExtendedTextInputTraits selectionBarColor]): Deleted. Remove some old versions of the `UIAsyncTextInput` implementation that are no longer necessary, since they've all been superceded by `BETextInput`. Canonical link: https://commits.webkit.org/277233@main

webkit-commit-queue · 2024-04-09T06:00:28Z

Committed 277233@main (697b1a8): https://commits.webkit.org/277233@main

Reviewed commits have been landed. Closing PR #27003 and removing active labels.

whsieh requested a review from cdumez as a code owner April 9, 2024 01:09

whsieh self-assigned this Apr 9, 2024

whsieh added the New Bugs Unclassified bugs are placed in this component until the correct component can be determined. label Apr 9, 2024

whsieh requested review from hortont424, pxlcoder, rr-codes, aprotyas and megangardner April 9, 2024 01:10

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Apr 9, 2024

whsieh removed the merging-blocked Applied to prevent a change from being merged label Apr 9, 2024

whsieh force-pushed the eng/271864 branch from 9254368 to 41e070e Compare April 9, 2024 01:26

pxlcoder approved these changes Apr 9, 2024

View reviewed changes

whsieh added the merge-queue Applied to send a pull request to merge-queue label Apr 9, 2024

webkit-commit-queue force-pushed the eng/271864 branch from 41e070e to 697b1a8 Compare April 9, 2024 06:00

webkit-commit-queue merged commit 697b1a8 into WebKit:main Apr 9, 2024

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Apr 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ iOS ] 3x editing/spelling/* (layout-tests) are constant failures #27003

[ iOS ] 3x editing/spelling/* (layout-tests) are constant failures #27003

whsieh commented Apr 9, 2024 •

edited by webkit-early-warning-system

Loading

webkit-early-warning-system commented Apr 9, 2024 •

edited

Loading

webkit-early-warning-system commented Apr 9, 2024 •

edited

Loading

pxlcoder Apr 9, 2024

whsieh Apr 9, 2024

pxlcoder Apr 9, 2024

whsieh Apr 9, 2024

whsieh commented Apr 9, 2024

webkit-commit-queue commented Apr 9, 2024

[ iOS ] 3x editing/spelling/* (layout-tests) are constant failures #27003

[ iOS ] 3x editing/spelling/* (layout-tests) are constant failures #27003

Conversation

whsieh commented Apr 9, 2024 • edited by webkit-early-warning-system Loading

697b1a8

webkit-early-warning-system commented Apr 9, 2024 • edited Loading

webkit-early-warning-system commented Apr 9, 2024 • edited Loading

pxlcoder Apr 9, 2024

Choose a reason for hiding this comment

whsieh Apr 9, 2024

Choose a reason for hiding this comment

pxlcoder Apr 9, 2024

Choose a reason for hiding this comment

whsieh Apr 9, 2024

Choose a reason for hiding this comment

whsieh commented Apr 9, 2024

webkit-commit-queue commented Apr 9, 2024

whsieh commented Apr 9, 2024 •

edited by webkit-early-warning-system

Loading

`697b1a8`

webkit-early-warning-system commented Apr 9, 2024 •

edited

Loading

webkit-early-warning-system commented Apr 9, 2024 •

edited

Loading