See https://bugs.webkit.org/show_bug.cgi?id=201531. This bug tracks the fix for the hardware keyboard. <rdar://problem/51404664>
Created attachment 378278 [details] Fixes the bug
Created attachment 378279 [details] Fixed non-iOS builds
Created attachment 378282 [details] GTK build fix attempt
Created attachment 378283 [details] Another GTK build fix attempt
Created attachment 378284 [details] Yet another GTK build fix attempt
Created attachment 378399 [details] Fixed build more
Comment on attachment 378399 [details] Fixed build more View in context: https://bugs.webkit.org/attachment.cgi?id=378399&action=review > Source/WebCore/ChangeLog:16 > + are involed from author scripts. Nit - involed => involved > Source/WebCore/editing/VisibleUnits.cpp:772 > + return findNextWordFromIndex(text, offset, NextWordDirection::Backward, iosMode); Nit - maybe name the variable nextWordMode or whitespaceMode instead of iosMode? (Or perhaps more verbose — nextWordModeInIOS). > Source/WebCore/editing/VisibleUnits.h:45 > +WEBCORE_EXPORT VisiblePosition nextWordPosition(const VisiblePosition &, NextWordModeInIOS = NextWordModeInIOS::LegacyStopBeforeWord); Nit - might consider removing the extra space before the ampersand in `const VisiblePosition &`, while we’re touching this line. > Source/WebCore/platform/text/TextBoundaries.h:51 > + enum class NextWordDirection { Forward, Backward }; I usually use `enum class NextWordDirection : bool { ~ }` for these, but it doesn’t really matter since NextWordDirection isn’t stored anywhere. > Source/WebCore/platform/text/TextBoundaries.h:52 > + enum class NextWordModeInIOS { LegacyStopBeforeWord, StopAfterWord }; (Ditto with the bool enum class)
Thanks for the review. (In reply to Wenson Hsieh from comment #7) > Comment on attachment 378399 [details] > Fixed build more > > View in context: > https://bugs.webkit.org/attachment.cgi?id=378399&action=review > > > Source/WebCore/ChangeLog:16 > > + are involed from author scripts. > > Nit - involed => involved Fixed. > > Source/WebCore/editing/VisibleUnits.cpp:772 > > + return findNextWordFromIndex(text, offset, NextWordDirection::Backward, iosMode); > > Nit - maybe name the variable nextWordMode or whitespaceMode instead of > iosMode? (Or perhaps more verbose — nextWordModeInIOS). Will use nextWordModeInIOS. whitespaceMode isn't right because we consider delimiters such as "," as well. > > Source/WebCore/editing/VisibleUnits.h:45 > > +WEBCORE_EXPORT VisiblePosition nextWordPosition(const VisiblePosition &, NextWordModeInIOS = NextWordModeInIOS::LegacyStopBeforeWord); > > Nit - might consider removing the extra space before the ampersand in `const > VisiblePosition &`, while we’re touching this line. Sure, done. > > Source/WebCore/platform/text/TextBoundaries.h:51 > > + enum class NextWordDirection { Forward, Backward }; > > I usually use `enum class NextWordDirection : bool { ~ }` for these, but it > doesn’t really matter since NextWordDirection isn’t stored anywhere. Thanks for catching that. I always forget to specify the enum class' width. I've been coding too long in old C/C++ :( > > Source/WebCore/platform/text/TextBoundaries.h:52 > > + enum class NextWordModeInIOS { LegacyStopBeforeWord, StopAfterWord }; > > (Ditto with the bool enum class) Fixed this one too.
Created attachment 378444 [details] Patch for landing
Comment on attachment 378444 [details] Patch for landing Wait for EWS again.
Finally!
Comment on attachment 378444 [details] Patch for landing Ugh... wrong + :(
Comment on attachment 378444 [details] Patch for landing Clearing flags on attachment: 378444 Committed r249709: <https://trac.webkit.org/changeset/249709>
All reviewed patches have been landed. Closing bug.
Reverted r249709 for reason: Layout test added in this patch has been consistently failing since it landed. Committed r249838: <https://trac.webkit.org/changeset/249838>