-
Notifications
You must be signed in to change notification settings - Fork 5.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[web] - Fix inputmode
on Android Firefox
#46901
Conversation
@@ -1936,7 +1936,7 @@ DefaultTextEditingStrategy createDefaultTextEditingStrategy(HybridTextEditing te | |||
strategy = IOSTextEditingStrategy(textEditing); | |||
} else if (browserEngine == BrowserEngine.webkit) { | |||
strategy = SafariDesktopTextEditingStrategy(textEditing); | |||
} else if (browserEngine == BrowserEngine.blink && | |||
} else if ((browserEngine == BrowserEngine.blink || browserEngine == BrowserEngine.firefox) && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a contextual reason why we have to specify a browser engine as well as the OS when we're trying to select for a mobile text editing strategy? Or can we just always choose the android strategy when OS == android
and always choose ios strategy when OS == iOS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes sense to remove the browser engine check here and see what happens.
One thing to keep in mind is that on iOS, all browsers have to use the Webkit engine under the hood (because iOS doesn't allow apps to ship their own JIT, and all browsers need a JIT, so they have to use Webkit from iOS). On Android, this isn't the case.
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold. |
983e18e
to
67b930b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
…143183) flutter/engine@fb99a84...6f5b6a3 2024-02-08 110993981+htoor3@users.noreply.github.com [web] - Fix `inputmode` on Android Firefox (flutter/engine#46901) 2024-02-08 bdero@google.com [Impeller] Vulkan: Don't fail initialization if stencil-only textures aren't supported. (flutter/engine#50455) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC chinmaygarde@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
The wrong keyboard type shows when users specify the phone input type in the framework.
This is happening because:
inputmode
for mobile text editing strategies.Fixes flutter/flutter#136351
Pre-launch Checklist
///
).