fix(modal): Fix modal to work when Content Security Policy is enabled

[farrago]

Overview

Updates modal directive so that it works with CSP enabled.

Details

Due to the use of inline styles, the modal does not work when CSP is enabled (at least without unsafe-inline which is obviously unsafe).

This is fixed by:

• [modal.js] Replace use of angular.element.attr({style: ...}) with angular.element.css({...}). The former is considered unsafe inline styles.
• [modal.js] Entirely remove initial setting of style after the modal-window element is created.
  • it is replaced immediately after a $timeout, so isn't meaningful
  • even if .css() is used before $compile, it is turned into a style which is then merged with the template by angularjs in the $compile, turning into an unsafe-inline style.
  • if .css() is used immediately after the compile, the styles are lost again when the directive's template loads asynchronously and updates the element.
  • so, using .css() after the $timeout is the only place we can safely and effectively add styles.
• [modal.js & backdrop.html] Remove inline style in the template, and instead apply display: block using .css() after the $timeout.
  • The inline style in the template is considered unsafe
  • .css() must be after $timeout for reasons above
  • See Backward compatibility below for more on why this replacement was chosen
• [window.html] Remove inline style in the template
  • The same styles were also being set by js anyway so doesn't do anything additional.

Backwards Compatibility

Replacing backdrop.html inline style with style applied via JS

The ideal replacement for the inline style would be a class with associated css so it could be over-ridden in end-user templates.

However, angular-foundation-6 doesn't include it's own css and it would be a breaking change to to require apps pull that for a new version.

So the options were:

1. apply the style in js using .css() (which is csp safe)
2. leave the inline style in place, and require that the end-user overrides the template with their own one which uses a class and css to achieve CSP support.
3. as (1), but offer a new attribute to disable that behaviour

Option (1) was chosen in the PR because:

• the use of display:block on the background is implicit in how the dialog is positioned etc. So it really has to be there, excluding significant effort on the client side.
• it makes the modal automatically work in sites with CSP enabled - less chance of end-user thinking modals are just broken in general
• it seems unlikely that end-users are doing significantly different things with the template such that adding the extra attribute is worth it.

This may be a breaking change for a very small number of users, but I find it unlikely and makes it more likely to just work for future users.

Other changes

The other changes should not affect backwards compatibility in any way. Obviously it doesn't fix any inline styles in users' own templates that they may have copied prior to this version.

Test Plan

(Using the CSP testing updates from the previous PR)

• Run gulp (WITHOUT CSP headers), and check:
  • All tests run and pass
  • All modal buttons on the demo page work and all modals show correctly
• Run gulp --csp (WITH CSP headers), and check:
  • All tests run and pass
  • All modal buttons on the demo page work and all modals show correctly
  • No further CSP errors are reported (other parts of the page report errors).
• Run gulp --csp --modules=modal (WITH CSP headers, modal only), and check:
  • No CSP errors at all are reported (to check the other modules aren't masking some errors)

[farrago]

Please especially check that you are happy with my choice in the backwards compatibility section. If you think one of the other choices would be more appropriate then I'm happy to make the change.

I also expect to have PR soon with fixes for the menus: I think that is purely with the demo html having an inline style, and the menu directive itself is ok.

[circlingthesun]

Looks good to me. 👍