-
Notifications
You must be signed in to change notification settings - Fork 27.4k
fix(inputs): ignoring input events in IE caused by placeholder changes #9697
Conversation
@jbedard want to add a few variations to this test? This only captures a tiny subset of the issues I think. Anyways, if travis is green I want to land this, but I'm going to be out for a few hours, so you have some time to add stuff if needed |
@IgorMinar we don't seem to be running e2e tests on ie anyways, so this doesn't seem supper helpful. Adding another browser would probably cause zillions of flakes |
var supportInputEvent = 'oninput' in document.createElement('div'); | ||
|
||
if (msie < 12) { | ||
// IE10/11 have many bugs with input event, don't use it. |
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.
Shouldn't IE9 be mentioned here ?
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.
Ie 9v doesn't support input events
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.
In sniffer.js
and its spec, it is implied that it does implement input
events, we just prefer to ignore them, because the implementation is so fubared.
Either way (no implementation vs fubared&ignored) it probably makes sense to add a comment here, so future "touchers" don't wonder: "Hm...did we miss IE9 here ?"
I can add more hopefully later today. Should we remove the unit tests and replace them all with e2e? |
Yeah probably |
@juliemr what do these mean:
It's pretty hard to understand that error :( |
That test is testing the docs application - compared to the examples in the docs |
Yes, but the problem is that it's failing, we need it to not fail. If we don't care about IE11 errors in the docs app, then we can conditionally not run IE for it. Otherwise we need to figure out what specifically is failing, and fix it |
I suspect that the e2e middleware might be messing with the test? |
Or not :-) |
So I guess the problem is that the page is not changing correctly when you click on the ngClick link on the home page. |
there are like at least 5 docs app failures in IE11, which is not a great sign |
Question is - is the app failing because it is broken on IE or is Protractor failing because webDriver on IE is bit rubbish? |
Let me boot up my Win8 machine |
I think the app is at least usable in IE11 --- but it's possible that the tests aren't testing very important things. It worked on SL last time I tried it |
If I go to the docs site (in IE11) and click on the |
Could be an issue with webdriver |
could also just be mis-configured, not sure exactly |
@juliemr is my protractor configuration (for adding IE) just really wrong, or do you think these are legitimate failures? I'm not sure |
ping @juliemr |
Sorry, the ping hit me in the middle of "aaaah conferences ignore all email". Let me know if there's still any issues I could help with. |
well I still haven't gotten e2e tests running on IE If you could have a go at getting tests running on IE, it would be good. It doesn't need to merge today though |
@juliemr we still really need to get these tests running on IE --- I haven't been able to get that working during my attempts, so it would be really good to get some help with that. |
No description provided.