fix(win8): Make Angular work on Windows 8 JS Apps

[panarasi]

Browser: Other
Component: misc core
Regression: no

Make Angular JS work on Windows 8 JavaScript apps.

In order to ensure the right level of security, the Windows platform prevents certain things to be done in JavaScript code, therefore developers have to make some adjustments to their Web code.
The changes in the pull request for AngularJS are in line with Windows 8 recommendations without breaking the Web.
All the unit test cases for the web have been run. The scenario runners have also been run on Windows Store apps to ensure that these changes work.

With these changes, developers can use AngularJS for Windows Store apps the exact same way they do for their Web apps.

[mary-poppins]

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

• Uses the issue template (#6217)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

[caitp]

So, a couple of things about this:

1. With branch prediction, the code is probably not that big of a deal --- however this could be patched one time and therefore not need to rely on branch prediction optimizations.

2. It should be totally possible to factor out windows phone security requirements into a separate module, it really comes down to just monkey-patching

3. We aren't running any tests on windows mobile devices. It would be good if we were, but currently we aren't.

Still, I applaud the effort to get Angular behaving better with certain mobile devices, so maybe there's a chance this could be done.

[panarasi]

@caitp This is not for Windows Phone, but for Windows 8 apps written in HTML and JavaScript.

[panarasi]

@caitp Thanks for the feedback.

Windows Store apps do not yet support Karma or protractor yet. We are looking at using Cordova's Medic to see if we could automate tests for the Windows 8 Store apps.

[caitp]

This is known to affect WP8, @panarasi. At any rate, if this makes it into angular core, I think it will need to be shaped differently.

[panarasi]

@caitp How do you think we should shape it ? I have created a thread on the mailing list too, for the discussion.

[caitp]

If WP8 is detected, function looks like this ... otherwise function looks like this ...

Pretty straight forward monkey patching. This could be put in a separate module if it ends up being a lot of code, too, but that leads to some difficulties.

I think jQuery suffers from these same problems as I don't see anything in jQuery master branch which gets around this.

[IgorMinar]

@tbosch @shepheb do you guys have any idea about how much work it would be to start testing on Windows 8?

would we somehow need to create a fake container for karma and webdriver or is this somehow already solved at webdriver level or on the OS level?

[caitp]

So I've discussed this a bit with Igor, and I think what we really need for proper WP8 support is a good testing strategy. It would be great if you (or your office, or whoever) could enhance this PR with some tests.

Looking at the PR again, I think this will hurt performance a bit on WP8 devices, but maybe that can't be helped, so you can probably ignore the issues I've mentioned about it.

Probably should take the docs changes out of the diff, though.

If anyone can help you get WP8 stuff running on CI builds, that is probably welcome.

[panarasi]

@caitp Question about WP8 - when you say WP8, do you mean the browser (IE) on Windows Phone 8 has issues ?
The pull request was meant to be only for Windows 8 Store apps and it uses constructs like MSApp.execUnsafeLocalFunction which are specific to Windows Store apps.

[panarasi]

@IgorMinar Windows 8 does not have a web driver yet and hence, running on protractor would be hard. For Karma, I am thinking of a karma launcher that would do the following

1. Save context.html, the test files, the jasmine spec source files and other resources to the local file system.
2. Wrap these files in a Windows Store app, and launch the app.
3. Given that cross domain requests are possible on Windows Store apps, the results would be communicated with the Karma server
4. The Windows app will be started and stopped just like it does in the Cordova testing framework.

Alternatively, we could write a karma framework that saves the files, and then a karma launcher, if step 1 seems dirty.

[caitp]

@panarasi: http://msdn.microsoft.com/en-us/library/windows/apps/hh465380.aspx these affect windows phone apps in general, it's their platform.

Now, I have seen some information about running webdriver on WP8 --- http://winphonewebdriver.codeplex.com/ however it appears not to have been updated in a couple months, so I'm not sure how far we could get with that.

But it's a start.

[petebacondarwin]

I'm pretty sure the guys at http://www.thinktecture.com/ were wrapping angularjs apps as Windows 8 apps.

[IgorMinar]

just like we have msie variable defined in Angular.js file, could you also define msApp or something similar? that would simplify the checks throughout the code base.

[panarasi]

Sounds good. I was hoping to not do browser (or in this case, platform detection). Do you suggest that I change all the conditional checks to detecting a centralized MsAPP app, just like msie ?

[panarasi]

@caitp This link from your comment is for a Windows Store app, it is not for a Windows Phone 8 app.

The programming model for Windows 8 Store apps and Windows Phone 8 apps are different. I am sorry I still do not understand how this impacts Windows Phone 8 apps, can you please elaborate ?

Both platforms use this technology, and we've had bug reports for this exact same issue regarding windows phone apps. If they don't, then the bug reports don't really make a lot of sense. In either case it's unimportant.

[panarasi]

@petebacondarwin Is there a specific link that we can look at, where they wrap Angular for a Windows Store app?

[petebacondarwin]

@panarasi - no but I am sure I heard them talking about it at a conference. You could try sending them an email - they're very friendly.

[IgorMinar]

@panarasi I think the "WP8" confusion is just a terminology issue on our part. We weren't aware that WP8 and Windows Store app were two different things :-) now we know...

[panarasi]

@caitp Monkey Patching may not work as we have properties like innerHTML that could be problem

[panarasi]

@IgorMinar I am working on replacing all the checks with the msapp variable as you have mentioned.

I am also working on getting a karma-windows8app-launcher, but it may take some time. In the meantime, I have been able to run is a Windows App that runs the various docs-scenario.html test cases and has the same pass rate as desktop IE.

Are there any other things that are needed before this pull request can be merged in ?

[mary-poppins]

I'm sorry, but I wasn't able to verify your Contributor License Agreement (CLA) signature. CLA signature is required for any code contributions to AngularJS.

Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match.

If you signed the CLA as a corporation, please let us know the company's name.

Thanks a bunch!

PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR.
PS2: If you are a Googler, please sign the CLA as well to simplify the CLA verification process.

[caitp]

this seems redundant, why not just ask for MSApp then?

[panarasi]

@IgorMinar wanted an msapp variable that was checked. @caitp Are you saying that we should get rid of this line and look only for MSApp at all places where we are doing the checks?

[mary-poppins]

CLA signature verified! Thank you!

Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes).

[old9]

Seems that it needs even more patches, like here , here and here

[panarasi]

@old9 Can you please explain ? Did you run these on the Windows 8 apps? Do you still see errors?

[old9]

Yeah I'm currently running an Angular app on windows 8, and it does report errors on those points.

Actually, this happens (only?) when I'm using jQuery(v2.0.3) together with Angular.

I'm not quite sure if it's the right place to apply the wrap patch, these are just my guesses from the error traces: for the first link, it calls the originalJqFn.apply directly without an execUnsafeLocalFunction wrapper, and the second it calls replaceChild in line 1925 and so on.

[panarasi]

@old9 Given that jqLite is bundled with Angular, fixing that makes sense. However, if these issues are fixed in jQuery, we would have to fix them here.

Note that jQuery had Windows Store apps in 2.0+. This error could be a regression in JQuery?

[old9]

@panarasi Yes they did claim a support for Windows Store apps, but I haven't had any lucky on any 2.0+ versions with Angular, so I think there might be more things to tackle on the Angular side, since Angular does something different itself when jQuery is detected.

Besides, some of the errors only complain about angular.js rather than jquery.js:
this error trace happens on directives with replace:true and template with ng-* attributes:

at replaceWith (ms-appx://1348fbed-5052-4ed7-9715-fcf3cc930a04/scripts/angular.js:6733:9)
at Anonymous function (ms-appx://1348fbed-5052-4ed7-9715-fcf3cc930a04/scripts/angular.js:6511:13)
at Anonymous function (ms-appx://1348fbed-5052-4ed7-9715-fcf3cc930a04/scripts/angular.js:7711:11)
at wrappedCallback (ms-appx://1348fbed-5052-4ed7-9715-fcf3cc930a04/scripts/angular.js:11077:15)
at wrappedCallback (ms-appx://1348fbed-5052-4ed7-9715-fcf3cc930a04/scripts/angular.js:11077:15)
at Anonymous function (ms-appx://1348fbed-5052-4ed7-9715-fcf3cc930a04/scripts/angular.js:11163:11)
at $eval (ms-appx://1348fbed-5052-4ed7-9715-fcf3cc930a04/scripts/angular.js:12106:9)
at $digest (ms-appx://1348fbed-5052-4ed7-9715-fcf3cc930a04/scripts/angular.js:11934:15)
at $apply (ms-appx://1348fbed-5052-4ed7-9715-fcf3cc930a04/scripts/angular.js:12210:13)
at done (ms-appx://1348fbed-5052-4ed7-9715-fcf3cc930a04/scripts/angular.js:7970:34)

[caitp]

That would be from here, not jqLite

[sgrebnov]

I've re-tested all the places mentioned by placing breakpoints there - all of them are called and work correct (including animation module) when jQuery is NOT used.
In case of jQuery (2.1.0) is used - it fails from the beginning - unable to add definitions to head

!angular.$$csp() && angular.element(document).find('head').prepend('<style type="text/css">@charset "UTF-8";[ng\\:cloak],[ng-cloak],[data-ng-cloak],[x-ng-cloak],.ng-cloak,.x-ng-cloak,.ng-hide{display:none !important;}ng\\:form{display:block;}.ng-animate-block-transitions{transition:0s all!important;-webkit-transition:0s all!important;}</style>');

[sgrebnov]

Added special logic to tweak jQuery to get rid of Windows Store restrictions. Tested with jquery 2.1.0.
Please review.
What jquery versions do we officially support? I'll review if additional changes are required.

[IgorMinar]

@sgrebnov Angular 1.2.x officially supports jQuery 1.10.x. Official support for jQuery 2.x is coming with Angular 1.3.

[chirayuk]

Hi Parashuram,

I took a look at PR 6217.  The changes I see boil down to these:

• A change to $$sanitize to customize the aHrefSanitizationWhitelist and imgSrcSanitizationWhitelist.
• A change to $browser to support ms-appx:// URLs in addition to http/https URLs.
• A change to jQLite to alter the meaning of html().  (domManip appears unrelated to Angular so I'm ignoring it.)

Customizing aHrefSanitizationWhitelist and imgSrcSanitizationWhitelist

Angular provides hooks to let you customize these two lists so you don't need to modify Angular for this.  Use something like this:

````js
angular.module('myApp', []).config(function($compileProvider) {
  $compileProvider.aHrefSanitizationWhitelist(
      /^\s*(https?|ftp|mailto|tel|file|ms-appx|ms-appdata|x-wmapp\d):/);
  $compileProvider.imgSrcSanitizationWhitelist(
      /^\s*(https?|ftp|file|blob|ms-appx|ms-appdata|x-wmapp\d):|data:image\//);
});

Refer to the docs at $compileProvider

Customizing $browser to support ms-appx:// URLs

This code could be wrong / introduce security issues.  I don't know the implications for ms-appx:// URLs.  The browsers Same Origin policy / security model varies by protocol.

baseHref is used to set the cookie path.  As an example, the cookie policy for the file:// protocol varies from browser to browser and from browser version to version.  e.g. What's the origin for file:///c:/1/2?  Is it c: (so everything in C: shares the cookies)?  Or the parent directory of the file that's served?  It varies by browser and version.  These same types of questions would apply to ms-appx:// URLs as well – perhaps the browser also uses part of the path in addition to the fake domain for cookie isolation.  We don't know.  The change in protocol implies that Angular doesn't understand the security model and we don't want to make the wrong assumptions.

RFC 2109 defines cookies strictly in the context of HTTP.  Accordingly, the $browser code in Angular specifically handles only HTTP(s) URLs.

Altering the meaning of html() in JQLite

html() is supposed to innerHTML the result without sanitization.  (ng-bind-html is a directive that sanitizes or innerHTML's trusted text.)  window.toStaticHTML performs sanitization.  This isn't a bad thing, but it means that the meaning of the function has changed – both for public users as well as Angular internal.

I'm curious – do you really need this code?  Is it only for testing?  I expect that you don't need this is production.

My thoughts

It's great that you're using Angular for the Windows Store.  However, from the perspective of the pull request, I would like to keep this code outside of Angular.  This is similar to the pull requests that wanted to support chrome:// / chrome-extension:// URLs for Chrome apps/extensions that also were not accepted into Angular core.  There are many different protocol schemes that are not part of the standard and change at their own rate with their own implications on usage.  Getting it right and keeping it updated is probably best done outside the core code.

In order to use Angular in the Windows Store code, you could try this:

• switch to using $compileProvider.aHrefSanitizationWhitelist and $compileProvider.imgSrcSanitizationWhitelist as I showed earlier.
• for the baseHref in $browser.  I'm assuming that you need this patch (i.e. you do have a <BASE> tag in your application)  Since cookiePath is stored in a closure, it isn't easily fixed from outside code (e.g. via the decorator pattern).  For now, perhaps you could replace the $browserProvider with your own fork that supports ms-appx URLs as you've done.

Let me know if you run into hiccups along the way.  If you see SCE / Strict Contextual Escaping related issues, I can help with that.  Additionally, it might make sense to replace/decorate Angular's $sanitize service with one that uses/adds window.toStaticHTML().

Chirayu.

[panarasi]

Closing this pull request as per @chirayuk's comment