Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ShaneK
left a comment
ShaneK
left a comment
There was a problem hiding this comment.
Looks good to me, solid fix! I left a question about prefilled values getting cleared too, but I don't think it needs to hold this up.
|
|
||
| const check = () => { | ||
| if (nativeInput.value !== getValue()) { | ||
| onValueChange(nativeInput.value); |
There was a problem hiding this comment.
Is this meant to clear values the app set too, and not just what the user typed? Since autocomplete defaults to "off" and the inner input never gets a value attribute, WebKit treats every field as sensitive and resets it to "" on a back/forward restore. With value set to 'Saved from API' and nothing typed, on main the field goes blank but value keeps 'Saved from API' and nothing fires. With this PR value becomes "" and ionInput fires once with "", so an Angular FormControl or Vue v-model filled from an API ends up empty and dirty after going back, even on fields the user never touched. It's the same for ion-searchbar and password inputs. I couldn't get a real Safari restore going, so this is based on what WebKit's reset() does.
The PR body does say value becomes "", so this might be intended, in which case I think the prefill case should be called out too. I'm also not sure about resolves #30017, since that reporter wanted the value kept rather than cleared. What do you think?
| this.clickController?.handleClickCapture(ev); | ||
| } | ||
|
|
||
| @Listen('pageshow', { target: 'window' }) |
There was a problem hiding this comment.
| @Listen('pageshow', { target: 'window' }) | |
| /** | |
| * Safari clears password and autocomplete="off" fields after a | |
| * back/forward cache restore without firing `input`, so the value | |
| * is synced with the field when the page is shown again. | |
| */ | |
| @Listen('pageshow', { target: 'window' }) |
Nit: the handlers above this one all say why they exist, and a window pageshow listener for a Safari back/forward cache quirk isn't obvious from the component alone. The searchbar copy could use the same comment. Up to you!
Issue number: resolves #30017
What is the current behavior?
In Safari, going back to a page restored from the back/forward cache clears
ion-inputandion-searchbar, butvaluekeeps the typed text and noionInputfires. Angular forms, Vuev-modeland React state stay out of sync with what's on screen. Both components renderautocomplete="off"by default, which is what makes Safari clear them.What is the new behavior?
valuenow stays in sync with the field after a restore, as it already did in Chrome and Firefox. Unlike them, Safari still clears the field, sovaluebecomes""andionInputfires with it.pageshow, usually before the next frame but later with Web Inspector open, so the field is checked each frame for up to 500 ms after a restore.Does this introduce a breaking change?
Other information
With Web Inspector open, Safari clears the field one frame later than usual. Without it, the clear lands before the first frame after
pageshow, so a single check on the next frame passed every test but missed the clear whenever the inspector was open. The latest clear we saw was the second frame, so 500 ms leaves plenty of margin.