chore(travis): test on IE Edge #14401

Narretz · 2016-04-09T12:28:01Z

Narretz · 2016-04-09T15:31:45Z

Edge times out on one single test in the whole suite! And it makes no sense why exactly this test. There are also some input and ngTouch failures( the latter actually abort the test run completely ...)

jdalton · 2016-04-13T23:41:42Z

Hey @Narretz!
Just a heads up this test/sniff should be revisited on the next Edge release.

We recently ran into the same-ish issue onboarding Angular into our Edge regression testing and filed an internal bug. The good news is it was patched recently 😎

AmazingJaze · 2016-04-14T22:29:38Z

test/ng/directive/inputSpec.js


-      helper.changeInputValueTo('10123-03');


@Narretz when you were preparing this change, were you able to determine the root cause of these inputSpec failures? I've been looking at these failures recently as part of the work @jdalton mentioned to include AngularJS tests in our Edge Regression testing. I have concluded that this line helper.changeInputValueTo('10123-03'); is failing to set the .value property on the HTML element in Edge, which is what results in the eventual failure, but I had to stop before getting to "why" the value property isn't getting set.

If there is an Edge bug behind this I'd love to make sure it gets filed.

@AmazingJaze I assumed that Edge simply doesn't allow setting years with more than 4 digits in all the date input elements, so when you enter 10123-03 etc. the input has no value (because it's invalid) and we don't get a value to work with. But I haven't actually checked if this is the Edge bug.

It is indeed the case that Edge does not support dates with years past 9999.

The spec does explicitly allow years with more than 4 digits (but not all browsers support it).
See #13735 for some context.

Looking at this plnkr, http://plnkr.co/edit/TkRo7GD139Pab0qC5ZEm?p=preview it seems to be the case that there is no way to enter a year with more than 4 digits in Edge currently. In Chrome, I can always use the keyboard to input the parts of the date, and so I can also enter 5 digit dates. In Edge it looks like I can only select a date from the picker. Is this correct? This seems like a UX issue to me, and a bug in Edge.

AFAIK, it is indeed not possible to set the date without using the picker (unless you open up dev tools and set the value property directly). I guess it is a conscious choice (albeit one I'm not a big fan of 😛).

Thanks for the tips @Narretz and @gkalpak. I will follow up with the EdgeHTML folks internally about both of these issues regarding date inputs. Cheers 🍻

Narretz · 2016-04-14T23:00:18Z

Hi @jdalton thanks for the heads-up! Do you know when a new stable version is scheduled for release?

jdalton · 2016-04-14T23:16:34Z

Do you know when a new stable version is scheduled for release?

Not at the moment. I don't believe the patch has made it into our insider builds yet.

jdalton · 2016-04-18T19:48:44Z

@Narretz
Just to confirm the timeout is the same issue we patched do you have a screenshot of the fail/hang.

Narretz · 2016-04-19T12:53:25Z

@jdalton Here's the console output from such a failed test: https://s3.amazonaws.com/archive.travis-ci.org/jobs/121916115/log.txt (Search for "heartbeat")

AmazingJaze · 2016-04-22T18:26:20Z

@Narretz did you encounter any trouble getting $$cookieReader and $$cookieWriter tests to successfully pass on Edge?

When I try to run Angular.js tests in Edge, those tests would constantly fail for me, but I was able to get them working with a small change to the test code. Should I send a PR?

Narretz · 2016-04-25T20:21:53Z

@AmazingJaze No, there weren't any problems with the cookie services. But I was only able to test on microsoftedge 25.10586 (in Saucelabs) so maybe it's a problem introduced in later versions. You can open a PR and I'll take a look!

mgol · 2016-11-02T13:05:55Z

@Narretz there is no "IE Edge", it's just "Edge" or "Microsoft Edge"; commit messages should be updated.

Have you tried it again recently? Are there any further problems?

googlebot · 2016-12-01T11:37:47Z

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

googlebot · 2016-12-01T11:37:47Z

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

googlebot · 2017-02-15T16:24:49Z

CLAs look good, thanks!

googlebot · 2017-02-15T16:24:49Z

CLAs look good, thanks!

Narretz · 2017-04-04T14:06:14Z

Here's the last test that is currently failing in Edge which I don't know how to handle:

angular.js/test/AngularSpec.js

Line 1776 in 136a42a

    
           expect(allowAutoBootstrap(createFakeDoc({src: protocol + '//something'}, protocol))).toBe(true);

It concerns the auto-bootstrap prevention we built in.
The problem is that Edge doesn't set the origin property on a elements, at least not in Microsoft Edge 38.14393.0.0, EdgeHTML 14.14393.
MDN indicates that origin should be available, but it's unclear since which version: https://developer.mozilla.org/en-US/docs/Web/API/HTMLHyperlinkElementUtils/origin (check history of this page)

I assume we can try to test again when the Creators Update goes live later in April.

jdalton · 2017-04-04T17:02:52Z

@Narretz I don't believe that will be resolved with the Creators update since it's not resolved in the insider preview. I did find that this worked though:

new URL(someAnchorElement).origin

Narretz · 2017-04-05T10:00:31Z

Hi @jdalton , thanks for looking into this. Unfortunately in my local version, new URL(someAnchorElement).origin doesn't work for our use case, it returns null.
We need the origin to decide if we should allow auto-bootstrap of an AngularJS app in an extension context.

    var link = document.createElement('a');
    link.href = src.value // this is something like "ms-browser-extension://something";
    var url = new URL(link);
    // url.origin is null

So in short, if the link has the scheme ms-browser-extension:, origin returns null. It should return ms-browser-extension:// (I've also tested with all other vendor prefixed schemes and browserext:, none work in Edge)

Should I open an issue for that?

And regarding the origin prop on a elements, should I open a ticket too?

jdalton · 2017-04-05T17:30:27Z

And regarding the origin prop on a elements, should I open a ticket too?

I opened an internal issue for that, but an external to link to wouldn't hurt.

Should I open an issue for that?

Yes, for the new URL(...).origin not returning correctly for ms-extensions.

Regarding your detection. Isn't the protocol something you can use to as well when deciding to allow auto-bootstrapping?

Narretz · 2017-04-12T10:08:37Z

It seems like returning "null" from the scheme / protocol ms-browser-extension is in line with the spec: https://url.spec.whatwg.org/#concept-url-origin. Chrome and Firefox currently return ms-browser-extension:// and would then be in violation of the spec. It looks like they handle any non-special scheme as they do for http etc, whereas Edge treats everything like "Otherwise" except when it's special.

petebacondarwin · 2017-04-12T13:25:54Z

We need to check the origin because we want to allow autobootstrap within an extension only if the extension is loading the angular.js script from the same extension. It is not enough to know that we are in an extension. If we cannot identify the origin then cannot assert that we are not crossing from one extension to another.

The simple workaround (and best practice) is to always manually bootstrap AngularJS within an extension. This will always work on all browsers.

Until we can find a way to identify that the angular.js script is being loading into the same extension from which it is being sourced, we will not be able to allow auto bootstrapping within MS Edge extensions.

petebacondarwin

LGTM

mgol · 2017-04-13T14:57:58Z

karma-shared.conf.js

+        browserName: 'microsoftedge',
+        platform: 'Windows 10',
+        version: '14'
+      },


Can you also add a BrowserStack entry?

mgol · 2017-04-13T14:58:24Z

scripts/travis/build.sh

@@ -11,7 +11,7 @@ elif [ "$JOB" == "unit" ]; then
  if [ "$BROWSER_PROVIDER" == "browserstack" ]; then
    BROWSERS="BS_Chrome,BS_Safari,BS_Firefox,BS_IE_9,BS_IE_10,BS_IE_11,BS_iOS"
  else
-    BROWSERS="SL_Chrome,SL_Firefox,SL_Safari_8,SL_Safari_9,SL_IE_9,SL_IE_10,SL_IE_11,SL_iOS"
+    BROWSERS="SL_Chrome,SL_Firefox,SL_Safari_8,SL_Safari_9,SL_IE_9,SL_IE_10,SL_IE_11,SL_EDGE,SL_iOS"


The BrowserStack section can be extended as well.

mgol · 2017-04-13T20:18:53Z

Remember to change the commit message when landing! (you could also update the title of this PR to say just "Edge" instead of "IE Edge").

petebacondarwin · 2017-04-14T16:10:38Z

Feel free to follow @mgol's comment and merge this into master @Narretz

Closes angular#13687 Closes angular#14401

Closes #13687 Closes #14401

Narretz added the type: chore label Apr 9, 2016

googlebot added the cla: yes label Apr 9, 2016

Narretz force-pushed the chore-travis-ie-edge branch 5 times, most recently from a554311 to 875b84f Compare April 9, 2016 13:29

Narretz added this to the 1.5.x milestone Apr 9, 2016

Narretz mentioned this pull request Apr 9, 2016

Run travis tests on Microsoft Edge #13687

Closed

4 tasks

Narretz force-pushed the chore-travis-ie-edge branch 2 times, most recently from 327164e to 6c0c66d Compare April 13, 2016 22:49

AmazingJaze reviewed Apr 14, 2016
View reviewed changes

Narretz force-pushed the chore-travis-ie-edge branch from 6c0c66d to cd3125d Compare September 8, 2016 19:44

mgol mentioned this pull request Nov 29, 2016

feat(security): do not bootstrap from unknown schemes with a different origin #15428

Closed

Narretz force-pushed the chore-travis-ie-edge branch from cd3125d to c83df6a Compare December 1, 2016 11:37

googlebot added cla: no and removed cla: yes labels Dec 1, 2016

Narretz force-pushed the chore-travis-ie-edge branch from 6d8151c to adf5687 Compare February 15, 2017 16:24

googlebot added cla: yes and removed cla: no labels Feb 15, 2017

Narretz modified the milestones: 1.5.x, 1.6.x Mar 8, 2017

mgol mentioned this pull request Mar 24, 2017

docs(faq): clarify the versioning strategy #15845

Merged

2 tasks

Narretz force-pushed the chore-travis-ie-edge branch from adf5687 to 27a14b7 Compare April 4, 2017 14:03

Narretz modified the milestones: 1.6.5, 1.6.x Apr 4, 2017

Narretz force-pushed the chore-travis-ie-edge branch from 27a14b7 to eeb583c Compare April 4, 2017 14:49

petebacondarwin approved these changes Apr 13, 2017

View reviewed changes

mgol suggested changes Apr 13, 2017

View reviewed changes

Narretz and others added 3 commits April 17, 2017 16:36

chore: test on Microsoft Edge

d5b7dca

Closes angular#13687 Closes angular#14401

test(input): exclude tests that are only failing on Edge

40d1768

test(core): expect that Edge cannot auto-bootstrap in extensions

1ab05bf

Narretz force-pushed the chore-travis-ie-edge branch from 6ea1309 to 1ab05bf Compare April 19, 2017 10:13

Narretz merged commit 69c3faf into angular:master Apr 19, 2017

Narretz added a commit that referenced this pull request Apr 19, 2017

chore: test on Microsoft Edge

496138f

Closes #13687 Closes #14401

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(travis): test on IE Edge #14401

chore(travis): test on IE Edge #14401

Narretz commented Apr 9, 2016

Narretz commented Apr 9, 2016

jdalton commented Apr 13, 2016

AmazingJaze Apr 14, 2016

Narretz Apr 14, 2016 •

edited

Loading

gkalpak Apr 14, 2016

Narretz Apr 14, 2016 •

edited

Loading

gkalpak Apr 14, 2016

AmazingJaze Apr 15, 2016 •

edited

Loading

Narretz commented Apr 14, 2016

jdalton commented Apr 14, 2016

jdalton commented Apr 18, 2016 •

edited

Loading

Narretz commented Apr 19, 2016

AmazingJaze commented Apr 22, 2016 •

edited

Loading

Narretz commented Apr 25, 2016

mgol commented Nov 2, 2016

googlebot commented Dec 1, 2016

googlebot commented Dec 1, 2016

googlebot commented Feb 15, 2017

googlebot commented Feb 15, 2017

Narretz commented Apr 4, 2017

jdalton commented Apr 4, 2017

Narretz commented Apr 5, 2017 •

edited

Loading

jdalton commented Apr 5, 2017

Narretz commented Apr 12, 2017 •

edited

Loading

petebacondarwin commented Apr 12, 2017

petebacondarwin left a comment

mgol Apr 13, 2017

Narretz Apr 19, 2017

mgol Apr 13, 2017

mgol commented Apr 13, 2017

petebacondarwin commented Apr 14, 2017

chore(travis): test on IE Edge #14401

chore(travis): test on IE Edge #14401

Conversation

Narretz commented Apr 9, 2016

Narretz commented Apr 9, 2016

jdalton commented Apr 13, 2016

AmazingJaze Apr 14, 2016

Choose a reason for hiding this comment

Narretz Apr 14, 2016 • edited Loading

Choose a reason for hiding this comment

gkalpak Apr 14, 2016

Choose a reason for hiding this comment

Narretz Apr 14, 2016 • edited Loading

Choose a reason for hiding this comment

gkalpak Apr 14, 2016

Choose a reason for hiding this comment

AmazingJaze Apr 15, 2016 • edited Loading

Choose a reason for hiding this comment

Narretz commented Apr 14, 2016

jdalton commented Apr 14, 2016

jdalton commented Apr 18, 2016 • edited Loading

Narretz commented Apr 19, 2016

AmazingJaze commented Apr 22, 2016 • edited Loading

Narretz commented Apr 25, 2016

mgol commented Nov 2, 2016

googlebot commented Dec 1, 2016

googlebot commented Dec 1, 2016

googlebot commented Feb 15, 2017

googlebot commented Feb 15, 2017

Narretz commented Apr 4, 2017

jdalton commented Apr 4, 2017

Narretz commented Apr 5, 2017 • edited Loading

jdalton commented Apr 5, 2017

Narretz commented Apr 12, 2017 • edited Loading

petebacondarwin commented Apr 12, 2017

petebacondarwin left a comment

Choose a reason for hiding this comment

mgol Apr 13, 2017

Choose a reason for hiding this comment

Narretz Apr 19, 2017

Choose a reason for hiding this comment

mgol Apr 13, 2017

Choose a reason for hiding this comment

mgol commented Apr 13, 2017

petebacondarwin commented Apr 14, 2017

Narretz Apr 14, 2016 •

edited

Loading

Narretz Apr 14, 2016 •

edited

Loading

AmazingJaze Apr 15, 2016 •

edited

Loading

jdalton commented Apr 18, 2016 •

edited

Loading

AmazingJaze commented Apr 22, 2016 •

edited

Loading

Narretz commented Apr 5, 2017 •

edited

Loading

Narretz commented Apr 12, 2017 •

edited

Loading