Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

ng-model is not updated immediately in Chrome WebView or Browser #5308

Closed
davgothic opened this issue Dec 6, 2013 · 32 comments
Closed

ng-model is not updated immediately in Chrome WebView or Browser #5308

davgothic opened this issue Dec 6, 2013 · 32 comments

Comments

@davgothic
Copy link

On Android 4.4 (KitKat) using both the Chrome WebView (v30) and the Chrome Browser (v31.0.1650.59) an input with an ng-model is not updating the variable value until the focus leaves the input.

The following commit for version 1.2.2 is responsible for this bug:

If I revert that commit in angular 1.2.3 or revert to angular 1.2.1 the problem goes away.

Here is a demo of the problem:
http://plnkr.co/edit/ZcksakM9icREgxe38Pnf?p=preview

@caitp
Copy link
Contributor

caitp commented Dec 6, 2013

Do you think this is related to #5298 @davgothic? It doesn't look like it should be as it's not preventing default, but even so it's something to look at I guess (it's possible the browser compositionstarts regardless of IME) --- Yeah, it shouldn't matter WRT the other issue, but it's still interesting.

@davgothic
Copy link
Author

@caitp I did at first think this issue was related to #5298 but I rolled back angular to 1.2.0 (before the offending commit) and this issue is fixed but the other one is not.

@caitp
Copy link
Contributor

caitp commented Dec 6, 2013

Yeah, they are separate issues certainly (which is too bad, it would be easier to fix just one!)

Do you have any idea how we can get around this for languages/IMEs that don’t really “need” composition events? It seems like not a good idea

Caitlin Potter
snowball@defpixel.com

On Dec 6, 2013, at 10:17 AM, David Hancock notifications@github.com wrote:

@caitp I did at first think this issue was related to #5298 but I rolled back angular to 1.2.0 (before the offending commit) and this issue is fixed but the other one is not.


Reply to this email directly or view it on GitHub.

@caitp
Copy link
Contributor

caitp commented Dec 10, 2013

So, I'm not sure there is anything we can do about this :( Based on https://code.google.com/p/chromium/issues/detail?id=118639, it looks like we're pretty much at the mercy of what a given IME decides to tell us, and have no ability to detect a specific IME and change our behaviour in response.

I'm not sure what the right approach is, is an app usable at all on Android with this issue? Like, do you see the model updated just late? It might be the most sensible approach to just hope that IME vendors fix their bugs, rather than choosing to break apps which make use of them totally.

@tbosch
Copy link
Contributor

tbosch commented Dec 12, 2013

Quick update: I tried to reproduce this with Android Emulator and Chrome Shell, but it worked well.
My Nexus 5 should arrive in two days, so I can test it there.

Sorry, but this won't make it into the release today, but hopefully into the release next week.

@fabiobiondi
Copy link

I confirm the same problem (Samsung S4 / Chrome / Android 4.2.2):
"an input with an ng-model is not updating the variable value until the focus leaves the input."

Angular 1.2.3 has the problem
Angular 1.2.1 is ok

@caitp
Copy link
Contributor

caitp commented Dec 14, 2013

We know the cause, it's because we're not updating the model until we get compositionend, and android IMEs unfortunately don't do the best job of emiting this at a right/appropriate time. The question is, is it fair to rollback the change for that and potentially upset people who require CJK IMEs or arabic or something, or not?

Like, that's kind of a hard call to make, it seems like this should probably be dealt with elsewhere (eg in Android / 3rd party IMEs). I'm not totally convinced it makes sense to roll back.

What else can we do.. Set a timeout after the compositionstart or something? That's just going to introduce flakiness and that's no good.

If you have any ideas of how to deal with this from within angular in a fair and balanced way, I'd love to hear it. But I'm not so sure such a way exists

@fabiobiondi
Copy link

Hi caipt...
thank you for your support.

I think it's very important find a way to fix it because now it's impossible update stuff while using an Android mobile keyboard and it'is unacceptable :(
For example, now I'm developing a auto-complete component and as you can imagine I need the model is updated (and it's not until i close the keyboard or click on the "confirm" button).

If you don't want to remove the feature might you give us a little patch that we can include in our projects?

Thanks in advance

@caitp
Copy link
Contributor

caitp commented Dec 14, 2013

I've personally been experiencing this behaviour on web apps (which aren't angular-based, such as github) on my android phone for ages now, and this is actually the closest I've come to really explaining why it happens! kind of funny.

I don't know, I don't like the idea of hurting people with CJK requirements or anything, but I agree that it's not really a good thing. It's just that there probably isn't much we can do about it from within Angular while still supporting complex IMEs

@tbosch
Copy link
Contributor

tbosch commented Dec 17, 2013

Hi @clkao,
could you verify that this is working as expected?

@tbosch
Copy link
Contributor

tbosch commented Dec 17, 2013

@caitp Thanks for the PR and it seems to work fine!
I tested using the following procedure on a US keyboard on Mac OSX to create compositonstart / compositionend events:

  1. press a vowel a long time. Mac OSX opens a popup to select the accent you want
  2. press left/right arrows to select an accent. This triggers the compositionstart event.
  3. hit enter. This triggers the compositionend event.

The problem that #4684 solved when adapted to this procedure is: after pressing the left/right key to select an accent, the scope value changed and e.g. a bound filter for a repeater got updated before the right accent was chosen with the enter key. However, the model value should not change until the correct accent was chosen.

Your PR does not break the fixed behavior of #4684, so I will merge it.

@caitp
Copy link
Contributor

caitp commented Dec 17, 2013

Cool --- yeah it's working well for me with Swype on Nexus too, so just hopefully it doesn't break any internationalization features, since I think it's only really been tested with latin IMEs

Sounds good

@tbosch
Copy link
Contributor

tbosch commented Dec 18, 2013

Just got my Nexus 5 and tested on the device. I see the problem now:

Chrome does auto correction of things that are typed. It fires a compositionstart event when a word is started and a compositionend event when the word is finished, e.g. by pressing space or selection an auto completion.

So even with the PR from @caitp the scope value is not updated until the end of the word! So if we want to have updates on every key press we need something else...

Need to think a bit more about this...

@caitp
Copy link
Contributor

caitp commented Dec 18, 2013

hmm, I'm actually not seeing that on my device with swype, the model seems to update as soon as the gesture is complete, whether it's a single tap or a short thing like "js". But that's a bit bothersome

We could try and update the model on compositionupdate, but that seems like a bad idea. On the other hand, is waiting for a complete word really that bad? Since it's mobile, fewer digests might be for the best

@tbosch
Copy link
Contributor

tbosch commented Dec 18, 2013

I think we need to revert the commit for #4684 and postpone that until PR #2129 is merged.
Then people could do <input type="text" ng-model-update-on="compositionend,change"> if they wanted to wait for the end of the composition (e.g. Chrome auto completion or IME input).

Sounds good?

@tbosch
Copy link
Contributor

tbosch commented Dec 18, 2013

If we revert the change, we need to reopen #4684 and link it to PR #2129!

@clkao
Copy link
Contributor

clkao commented Dec 18, 2013

@tbosch I am all for the more customizable #2129, but sadly that is a more complicated change and is scheduled for 1.3.x

The issue here is: mobile input correction being compositions but immediate updates are desired. However it's probably still unwanted intermediate changes for CJK IME. I think a good compromise is that we still send out updates for ascii characters even during composition (on keypress event), so it works as desired for all cases without configuration.

This should be a relatively small change, what do people think?

@tbosch
Copy link
Contributor

tbosch commented Dec 18, 2013

But there may be auto completion for Chinese in Chrome for Android as
well...

Sent from my iPhone

On Dec 17, 2013, at 9:24 PM, Chia-liang Kao notifications@github.com
wrote:

@tbosch https://github.com/tbosch I am all for the more customizable
#2129#2129,
but sadly that is a more complicated change and is scheduled for 1.3.x

The issue here is: mobile input correction being compositions but immediate
updates are desired. However it's probably still unwanted intermediate
changes for CJK IME. I think a good compromise is that we still send out
updates for ascii characters even during composition (on keypress event),
so it works as desired for all cases without configuration.

This should be a relatively small change, what do people think?


Reply to this email directly or view it on
GitHubhttps://github.com//issues/5308#issuecomment-30818065
.

@clkao
Copy link
Contributor

clkao commented Dec 18, 2013

It seems the mobile browsers and individual IME vary quite a lot. On my android 4.1 mobilesafari does no composition event at all for cjk input and english auto-correction, and chrome for android sends out compositionend whenever autocomplete results are chosen for CJK. Another way until #2129 is is to completely disable this on android (just need to make $sniffer export the android var)

clkao added a commit to clkao/angular.js that referenced this issue Dec 18, 2013
Workaround for chrome for android until angular#2129 is ready.

Closes angular#5308, angular#5323
@caitp
Copy link
Contributor

caitp commented Dec 18, 2013

I'm not sure I like the idea of requiring ng-model-update-on="comma-separated-list-of-events" for this, because you'd need this for generally every input if you wanted to support some particular phone, and that just seems awful.

I think @clkao's sniffer approach is better (although, it should probably also sniff the version of Webkit/Blink, as I've said before this bug was not reproducible at all for me on 534), and just hope that future IMEs do a better job of reporting compositionend appropriately.

Or alternatively, it could be decided that model updates on autocomplete are not that bad, as this means fewer digest cycles on low-powered devices.

@tbosch
Copy link
Contributor

tbosch commented Dec 18, 2013

Just a side note: checking the characters does not help in this case. When using a Chinese locale on a US keyboard we only get normal us characters until the compositionend event, which replaces the input's value with the Chinese characters.

@clkao
Copy link
Contributor

clkao commented Dec 18, 2013

@tbosch actually, in real browsers during composition you get keycode 229 on keydown and then the ascii keycode on keyup, which is what you referred to. Sadly both events send out keycode 0 on N5 chrome for (tested by @walkingice earlier today)

So this really comes down to what @caitp said about autocomplete (or auto-correction) and model updates. And in this case, based on the use of the API, the new builtin text service on android or chrome for android seem to decide that incomplete english texts aren't intended values.

@tbosch
Copy link
Contributor

tbosch commented Dec 18, 2013

Ok, thought about it a lot and I agree with @clkao that we should just disable this for Android. I double checked for iOS and it does not fire composition events for word completion suggestions.

To summarize:

  • When entering characters using IME browsers send intermediate input events while composing, which get replaced by the composition once it is done. In this case we don't want to update the model on the Angular scope as the intermediate characters don't make sense in most of the cases.
  • But: Chrome for Android also uses the composition events for suggesting a corrected / completed word. It does not fire compositionend until the end of the word. In this case we want to update the Angular scope immediately as the entered characters do make sense in most of the cases.

Any vetos?

@caitp
Copy link
Contributor

caitp commented Dec 18, 2013

Not necessarily a veto, BUT @clkao was saying on IRC earlier that there would be unintended data in the buffer/model value prior to #4684, so I'm not sure it's necessarily a good call to ignore this on Android (unless @clkao can confirm that this actually doesn't have this effect in the case of Android + Chrome)

@clkao
Copy link
Contributor

clkao commented Dec 18, 2013

@caitp Thanks for bringing that up! The native mobile safari on android 4.1 doesn't do composition events, so #4684 couldn't have fixed it anyway. Having this remained "broken" for android for now is probably better than this reverted and be broken everywhere else ;)

@tbosch
Copy link
Contributor

tbosch commented Dec 18, 2013

Ok, let's do it. I took the PR from @clkao and added a missing test. Will merge when ci is green...

@tbosch tbosch closed this as completed in 3dc1803 Dec 18, 2013
@tbosch
Copy link
Contributor

tbosch commented Dec 18, 2013

Thanks all, specially @caitp and @clkao!

jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
@rodrigozrusso
Copy link

Hi @caitp and @clkao,
The problem came back on Chrome 33.0.1750.170 with angular 1.2.15 using Nexus 5.

@tbosch
Copy link
Contributor

tbosch commented Apr 7, 2014

Hi @rodrigozrusso, could you create a new issue?

@clkao
Copy link
Contributor

clkao commented Apr 12, 2014

@rodrigozrusso It'd be great if you can also bisect the commit causing the regression and put it in the new issue.

@rodrigozrusso
Copy link

@clkao @tbosch ok! I'll do it.

@PezCoder
Copy link

PezCoder commented May 29, 2018

I had the same issue that the title of this issue & description says:

ng-model is not updated immediately in Chrome WebView or Browser

I was a little confused after reading this thread on how introducing ng-model-options fixes this issue.

Failed experiment using ng-model-options

I tried using ng-model with ng-model-options like so on the input.

<!-- someInput.html -->
<!-- Before -->
<input ng-model="someInput" />

<!-- After -->
<input ng-model="someInput" ng-model-options="{updateOn: 'keyup'}" />

My logic was maybe ng-model is not relying on the compositionstart & end events when using the above code since I'm explicitly telling it to rely on only the keyup event. But sadly it didn't worked for me.

Solution

I ended up adding an extra ng-keyup listener & it worked like I expected it to from the start: 🎉

<!-- someInput.html -->
<!-- Before -->
<input ng-model="someInput" />

<!-- After -->
<input ng-model="someInput" ng-keyup="updateSomeInput($event)" />
/*
  SomeInputController.js
  Note: don't pass the angular's form element to this function, as it won't have the updated value
*/
$scope.updateSomeInput = function(event) {
  // manually updating the model here
  $scope.someInput = event.target.value;
};

Hope it helps for people looking for the similar issue, Please let me know if there is a better solution.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.