Repository navigation
[CP] Refactor route focus node creation #149178
Description
Issue Link
Target
stable
PR Link
Changelog Description
Refactor route focus node creation
Impacted Users
iOS users on Flutter stable 3.22.0 and 3.22.1
Impact Description
Unable to focus on TextField or open the keyboard again after side pop from another screen on iOS.
Workaround
Stay on 3.19.6 or use tags 3.22.0-19.0.pre and above
Risk
low
Test Coverage
yes
Validation Steps
-
Flutter stable 3.22.0, 3.22.1; tag 3.22.0-18.0.pre and below
- run an app with a TextField on iOS
- tap into TextField
- keyboard opens
- navigate to a separate screen
- swipe back (this is important)
- tap into TextField
- keyboard does not open
-
Flutter tag 3.22.0-19.0.pre and above
- run an app with a TextField on iOS
- tap into TextField
- keyboard opens
- navigate to a separate screen
- swipe back (this is important)
- tap into TextField
- keyboard opens
Activity
techouse commentedon May 28, 2024
I tested these versions
jmagman commentedon May 28, 2024
@justinmc @chunhtai I'm not sure if this is the right thing to cherry-pick, or instead it should be to fix forward to #147607. Could you take a look at the regression and test it out?
justinmc commentedon May 29, 2024
It looks like the actual PR that we would cherry pick is #147607, is that right @chunhtai? Also, are we sure that that is what fixes #148573 (and/or #148867) @techouse?
chunhtai commentedon May 29, 2024
I think in order to cherry pick the reapply, you will need to cherry pick the revert first right?
it seems like the release only contains the problematic popscope commit but not the revert and reland? if so we can either cherry pick the revert or both the revert and reland, I am fine with either one.
chunhtai commentedon May 29, 2024
actually it may be better to cherry pick both because the breaking change is already out with the problematic popscope commit, we don't want them to do it twice
jmagman commentedon May 29, 2024
@chunhtai could you prep that cherry-pick then since it's relatively complicated, assuming @justinmc would approve it as worth a hotfix.
techouse commentedon May 29, 2024
As stated above in #149178 (comment), I tested with 11 different versions, including one before and one after the fix.
If required, I can do some additional testing, but it seems more than likely that this is the fix/cause.
I do agree with @jmagman that it's a spicy one. 🙈
chunhtai commentedon May 29, 2024
I am new to the process, may I know which branch I should create the cherry pick against?
chunhtai commentedon May 29, 2024
actually this looks weird, the stable doesn't have the popscope change, something else is at play.
chunhtai commentedon May 29, 2024
This should be the commit that fixes the issue
#147390
can you verify? if it indeed is, we should change the this issue title and description
cc @gspencergoog
techouse commentedon May 29, 2024
@chunhtai You're right! It does look like #147390 is the real fix. I somehow I compared it in reverse, 3.22.0-19.0.pre...3.22.0-18.0.pre instead of 3.22.0-18.0.pre...3.22.0-19.0.pre 🤦 and the sheer quantity of
PopScopes got me.[-][CP] Revert "add a new PopScope.onPopWithResultInvoke widget to replace PopScope.onPopInvoke"[/-][+][CP] Refactor route focus node creation[/+]chunhtai commentedon May 29, 2024
I think this issue is bad enough that should deserve a cp. @gspencergoog Do you think this is a low risk cp?
gspencergoog commentedon May 29, 2024
Yes, I consider this low risk. The
ListenableBuilderpart doesn't change functionality at all, and thewithExternalFocusNodepart is the bug fix, and has very similar functionality to the original code, but will not overwrite theskipTraversaland other attributes of the node when they are out of sync with the Focus widget. The effect of the change should be local to this widget and itsFocusScopeNode.JosephNK commentedon May 31, 2024
This issue seems to require a hotfix. It is a big problem that text fields do not work properly in released apps.
lukemmtt commentedon Jun 6, 2024
This PR should be closed, as the fix was successfully merged and released as a hotfix release as Flutter 3.22.2:
Thank you @techouse and all others who helped troubleshoot and fix this! 🚀
github-actions commentedon Jun 21, 2024
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of
flutter doctor -vand a minimal reproduction of the issue.