Forbid direct style manipulation (#6012)

* Forbid direct style manipulation

They must go through `setStyle` or `setStyles`.

* Lint

* Types

* Allow 3p to depend on style module

* Test fixes

3p/twitter.js

@@ -17,6 +17,7 @@
 // TODO(malteubl) Move somewhere else since this is not an ad.
 
 import {loadScript} from './3p';
+import {setStyles} from '../src/style';
 
 /**
  * Produces the Twitter API object for the passed in callback. If the current
@@ -50,10 +51,12 @@ function getTwttr(global, cb) {
 export function twitter(global, data) {
   const tweet = global.document.createElement('div');
   tweet.id = 'tweet';
-  tweet.style.width = '100%';
-  tweet.style.display = 'flex';
-  tweet.style.alignItems = 'center';
-  tweet.style.justifyContent = 'center';
+  setStyles(tweet, {
+    width: '100%',
+    display: 'flex',
+    alignItems: 'center',
+    justifyContent: 'center',
+  });
   global.document.getElementById('c').appendChild(tweet);
   getTwttr(global, function(twttr) {
     // Dimensions are given by the parent frame.

ads/adspirit.js

@@ -15,6 +15,7 @@
  */
 
 import {validateData} from '../3p/3p';
+import {setStyles} from '../src/style';
 
 /**
   * @param {!Window} global
@@ -27,7 +28,10 @@ export function adspirit(global, data) {
   i.setAttribute('data-asm-params', data['asmParams']);
   i.setAttribute('data-asm-host', data['asmHost']);
   i.setAttribute('class', 'asm_async_creative');
-  i.style.cssText = 'display:inline-block;text-align:left;';
+  setStyles(i, {
+    display: 'inline-block',
+    'text-align': 'left',
+  });
   global.document.getElementById('c').appendChild(i);
   const s = global.document.createElement('script');
   s.src = 'https://' + data['asmHost'] + '/adasync.js';

ads/google/adsense.js

@@ -15,6 +15,7 @@
  */
 
 import {validateData} from '../../3p/3p';
+import {setStyles} from '../../src/style';
 
 /**
  * Make an adsense iframe.
@@ -54,7 +55,11 @@ export function adsense(global, data) {
   }
   i.setAttribute('data-page-url', global.context.canonicalUrl);
   i.setAttribute('class', 'adsbygoogle');
-  i.style.cssText = 'display:inline-block;width:100%;height:100%;';
+  setStyles(i, {
+    display: 'inline-block',
+    width: '100%',
+    height: '100%',
+  });
   const initializer = {};
   if (data['experimentId']) {
     const experimentIdList = data['experimentId'].split(',');

ads/inmobi.js

@@ -30,7 +30,7 @@ export function inmobi(global, data) {
     onError: code => {
       if (code == 'nfr') {
         global.context.noContentAvailable();
-        document.getElementById('my-ad-slot').style.display = 'none';
+        document.getElementById('my-ad-slot').style./*OK*/display = 'none';
       }
     },
     onSuccess: () => {

build-system/dep-check-config.js

@@ -64,10 +64,10 @@ exports.rules = [
       '3p/**->src/log.js',
       '3p/**->src/types.js',
       '3p/**->src/string.js',
+      '3p/**->src/style.js',
       '3p/**->src/url.js',
       '3p/**->src/config.js',
       '3p/**->src/mode.js',
-      '3p/**->src/mode.js',
       '3p/polyfills.js->src/polyfills/math-sign.js',
       '3p/polyfills.js->src/polyfills/object-assign.js',
     ],

build-system/tasks/presubmit-checks.js

@@ -478,6 +478,12 @@ var forbiddenTerms = {
       'src/runtime.js',
     ],
   },
+  'style\\.\\w+ = ': {
+    message: 'Use setStyle instead!',
+    whitelist: [
+      'testing/iframe.js',
+    ],
+  },
 };
 
 var ThreePTermsMessage = 'The 3p bootstrap iframe has no polyfills loaded and' +

extensions/amp-a4a/0.1/amp-a4a.js

@@ -44,6 +44,7 @@ import {
   PublicKeyInfoDef,
 } from './crypto-verifier';
 import {isExperimentOn} from '../../../src/experiments';
+import {setStyle} from '../../../src/style';
 import {handleClick} from '../../../ads/alp/handler';
 import {AdDisplayState} from '../../../extensions/amp-ad/0.1/amp-ad-ui';
 
@@ -755,7 +756,7 @@ export class AmpA4A extends AMP.BaseElement {
               // Ensure visibility hidden has been removed (set by boilerplate).
               const frameDoc = friendlyIframeEmbed.iframe.contentDocument ||
                 friendlyIframeEmbed.win.document;
-              frameDoc.body.style.visibility = 'visible';
+              setStyle(frameDoc.body, 'visibility', 'visible');
               // Capture phase click handlers on the ad.
               this.registerExpandUrlParams_(friendlyIframeEmbed.win);
               // Bubble phase click handlers on the ad.

extensions/amp-access/0.1/amp-login-done-dialog.js

@@ -74,7 +74,7 @@ export class LoginDoneDialog {
   setStyles_() {
     const doc = this.win.document;
     const style = doc.createElement('style');
-    style.textContent = this.buildStyles_();
+    style./*OK*/textContent = this.buildStyles_();
     doc.head.appendChild(style);
   }
 

extensions/amp-ad/0.1/amp-ad-xorigin-iframe-handler.js

@@ -25,6 +25,7 @@ import {IntersectionObserver} from '../../../src/intersection-observer';
 import {viewerForDoc} from '../../../src/viewer';
 import {dev, user} from '../../../src/log';
 import {timerFor} from '../../../src/timer';
+import {setStyle} from '../../../src/style';
 import {AdDisplayState} from './amp-ad-ui';
 
 const TIMEOUT_VALUE = 10000;
@@ -130,7 +131,7 @@ export class AmpAdXOriginIframeHandler {
 
     // Set iframe initially hidden which will be removed on load event +
     // post message.
-    this.iframe.style.visibility = 'hidden';
+    setStyle(this.iframe, 'visibility', 'hidden');
 
     this.element_.appendChild(this.iframe);
     return timerFor(this.baseInstance_.win).timeoutPromise(TIMEOUT_VALUE,
@@ -140,7 +141,7 @@ export class AmpAdXOriginIframeHandler {
           user().warn('AMP-AD', e);
         }).then(() => {
           if (this.iframe) {
-            this.iframe.style.visibility = '';
+            setStyle(this.iframe, 'visibility', '');
           }
         });
   }

extensions/amp-analytics/0.1/transport.js

@@ -23,6 +23,7 @@ import {dev, user} from '../../../src/log';
 import {loadPromise} from '../../../src/event-helper';
 import {timerFor} from '../../../src/timer';
 import {removeElement} from '../../../src/dom';
+import {setStyle} from '../../../src/style';
 
 /** @const {string} */
 const TAG_ = 'amp-analytics.Transport';
@@ -131,7 +132,7 @@ export function sendRequestUsingIframe(win, request) {
   assertHttpsUrl(request, 'amp-analytics request');
   /** @const {!Element} */
   const iframe = win.document.createElement('iframe');
-  iframe.style.display = 'none';
+  setStyle(iframe, 'display', 'none');
   iframe.onload = iframe.onerror = () => {
     timerFor(win).delay(() => {
       removeElement(iframe);

extensions/amp-carousel/0.1/scrollable-carousel.js

@@ -69,10 +69,10 @@ export class AmpScrollableCarousel extends BaseCarousel {
     this.cells_.forEach(cell => {
       this.setAsOwner(cell);
       cell.classList.add('amp-carousel-slide');
-      cell.style.display = 'inline-block';
+      st.setStyle(cell, 'display', 'inline-block');
       if (cell != this.cells_[0]) {
         // TODO(dvoytenko): this has to be customizable
-        cell.style.marginLeft = '8px';
+        st.setStyle(cell, 'marginLeft', '8px');
       }
       this.container_.appendChild(cell);
     });

extensions/amp-fit-text/0.1/amp-fit-text.js

@@ -94,7 +94,7 @@ class AmpFitText extends AMP.BaseElement {
     const maxWidth = this.element./*OK*/offsetWidth;
     const fontSize = calculateFontSize_(this.measurer_, maxHeight, maxWidth,
         this.minFontSize_, this.maxFontSize_);
-    this.contentWrapper_.style.fontSize = st.px(fontSize);
+    st.setStyle(this.contentWrapper_, 'fontSize', st.px(fontSize));
     updateOverflow_(this.contentWrapper_, this.measurer_, maxHeight,
         fontSize);
   }
@@ -115,7 +115,7 @@ export function calculateFontSize_(measurer, expectedHeight, expectedWidth,
   // Binomial search for the best font size.
   while (maxFontSize - minFontSize > 1) {
     const mid = Math.floor((minFontSize + maxFontSize) / 2);
-    measurer.style.fontSize = st.px(mid);
+    st.setStyle(measurer, 'fontSize', st.px(mid));
     const height = measurer./*OK*/offsetHeight;
     const width = measurer./*OK*/offsetWidth;
     if (height > expectedHeight || width > expectedWidth) {
@@ -136,7 +136,7 @@ export function calculateFontSize_(measurer, expectedHeight, expectedWidth,
  * @private  Visible for testing only!
  */
 export function updateOverflow_(content, measurer, maxHeight, fontSize) {
-  measurer.style.fontSize = st.px(fontSize);
+  st.setStyle(measurer, 'fontSize', st.px(fontSize));
   const overflown = measurer./*OK*/offsetHeight > maxHeight;
   const lineHeight = fontSize * LINE_HEIGHT_EM_;
   const numberOfLines = Math.floor(maxHeight / lineHeight);

extensions/amp-iframe/0.1/amp-iframe.js

@@ -25,6 +25,7 @@ import {timerFor} from '../../../src/timer';
 import {user} from '../../../src/log';
 import {urls} from '../../../src/config';
 import {moveLayoutRect} from '../../../src/layout-rect';
+import {setStyle} from '../../../src/style';
 
 /** @const {string} */
 const TAG_ = 'amp-iframe';
@@ -273,7 +274,7 @@ export class AmpIframe extends AMP.BaseElement {
     iframe.name = 'amp_iframe' + count++;
 
     if (this.isClickToPlay_) {
-      iframe.style.zIndex = -1;
+      setStyle(iframe, 'zIndex', -1);
     }
 
     this.propagateAttributes(
@@ -384,7 +385,7 @@ export class AmpIframe extends AMP.BaseElement {
     if (this.placeholder_) {
       this.getVsync().mutate(() => {
         if (this.iframe_) {
-          this.iframe_.style.zIndex = 0;
+          setStyle(this.iframe_, 'zIndex', 0);
           this.togglePlaceholder(false);
         }
       });

extensions/amp-install-serviceworker/0.1/amp-install-serviceworker.js

@@ -29,6 +29,7 @@ import {listen} from '../../../src/event-helper';
 import {timerFor} from '../../../src/timer';
 import {toggle} from '../../../src/style';
 import {viewerForDoc} from '../../../src/viewer';
+import {setStyle} from '../../../src/style';
 
 /** @private @const {string} */
 const TAG = 'amp-install-serviceworker';
@@ -113,8 +114,8 @@ export class AmpInstallServiceWorker extends AMP.BaseElement {
       return;
     }
     // The iframe will stil be loaded.
-    this.element.style.display = 'none';
-    const iframe = /*OK*/document.createElement('iframe');
+    setStyle(this.element, 'display', 'none');
+    const iframe = this.win.document.createElement('iframe');
     iframe.setAttribute('sandbox', 'allow-same-origin allow-scripts');
     iframe.src = this.iframeSrc_;
     this.element.appendChild(iframe);

extensions/amp-install-serviceworker/0.1/test/test-amp-install-serviceworker.js

@@ -176,7 +176,10 @@ describe('amp-install-serviceworker', () => {
         },
         setTimeout: window.setTimeout,
         clearTimeout: window.clearTimeout,
-        document: {nodeType: /* document */ 9},
+        document: {
+          nodeType: /* document */ 9,
+          createElement: document.createElement.bind(document),
+        },
       };
       installTimerService(win);
       win.document.defaultView = win;

extensions/amp-lightbox/0.1/amp-lightbox.js

@@ -100,12 +100,14 @@ class AmpLightbox extends AMP.BaseElement {
     this.getViewport().enterLightboxMode();
 
     this.mutateElement(() => {
-      this.element.style.display = '';
-      this.element.style.opacity = 0;
-      // TODO(dvoytenko): use new animations support instead.
-      this.element.style.transition = 'opacity 0.1s ease-in';
+      st.setStyles(this.element, {
+        display: '',
+        opacity: 0,
+        // TODO(dvoytenko): use new animations support instead.
+        transition: 'opacity 0.1s ease-in',
+      });
       vsyncFor(this.win).mutate(() => {
-        this.element.style.opacity = '';
+        st.setStyle(this.element, 'opacity', '');
       });
     }).then(() => {
       const container = dev().assertElement(this.container_);

extensions/amp-slides/0.1/amp-slides.js

@@ -38,7 +38,7 @@ class AmpSlides extends AMP.BaseElement {
     this.slides_.forEach((slide, i) => {
       this.setAsOwner(slide);
       // Only the first element is initially visible.
-      slide.style.display = i > 0 ? 'none' : 'block';
+      st.setStyle(slide, 'display', i > 0 ? 'none' : 'block');
       this.applyFillContent(slide);
     });
 
@@ -119,7 +119,7 @@ class AmpSlides extends AMP.BaseElement {
       if (!animate) {
         this.commitSwitch_(oldSlide, newSlide);
       } else {
-        oldSlide.style.zIndex = 0;
+        st.setStyle(oldSlide, 'zIndex', 0);
         Animation.animate(this.element,
             this.createTransition_(oldSlide, newSlide, dir),
             200, 'ease-out').thenAlways(() => {
@@ -171,16 +171,20 @@ class AmpSlides extends AMP.BaseElement {
    * @private
    */
   commitSwitch_(oldSlide, newSlide) {
-    oldSlide.style.display = 'none';
-    oldSlide.style.zIndex = 0;
-    oldSlide.style.transform = '';
-    oldSlide.style.transition = '';
-    oldSlide.style.opacity = 1;
-    newSlide.style.display = 'block';
-    newSlide.style.zIndex = 0;
-    newSlide.style.transform = '';
-    newSlide.style.transition = '';
-    newSlide.style.opacity = 1;
+    st.setStyles(oldSlide, {
+      display: 'none',
+      zIndex: 0,
+      transform: '',
+      transition: '',
+      opacity: 1,
+    });
+    st.setStyles(newSlide, {
+      display: 'block',
+      zIndex: 0,
+      transform: '',
+      transition: '',
+      opacity: 1,
+    });
     this.scheduleLayout(newSlide);
     this.updateInViewport(oldSlide, false);
     this.updateInViewport(newSlide, true);

extensions/amp-user-notification/0.1/amp-user-notification.js

@@ -24,6 +24,7 @@ import {urlReplacementsForDoc} from '../../../src/url-replacements';
 import {viewerForDoc} from '../../../src/viewer';
 import {whenDocumentReady} from '../../../src/document-ready';
 import {xhrFor} from '../../../src/xhr';
+import {setStyle} from '../../../src/style';
 
 
 /** @private @const {string} */
@@ -276,7 +277,7 @@ export class AmpUserNotification extends AMP.BaseElement {
 
   /** @override */
   show() {
-    this.element.style.display = '';
+    setStyle(this.element, 'display', '');
     this.element.classList.add('amp-active');
     this.getViewport().addToFixedLayer(this.element);
     return this.dialogPromise_;

src/3p-frame.js

@@ -25,6 +25,7 @@ import {dashToCamelCase} from './string';
 import {parseUrl, assertHttpsUrl} from './url';
 import {viewerForDoc} from './viewer';
 import {urls} from './config';
+import {setStyle} from './style';
 
 
 /** @type {!Object<string,number>} Number of 3p frames on the for that type. */
@@ -123,8 +124,8 @@ export function getIframe(parentWindow, parentElement, opt_type, opt_context) {
   iframe.ampLocation = parseUrl(src);
   iframe.width = attributes.width;
   iframe.height = attributes.height;
-  iframe.style.border = 'none';
   iframe.setAttribute('scrolling', 'no');
+  setStyle(iframe, 'border', 'none');
   /** @this {!Element} */
   iframe.onload = function() {
     // Chrome does not reflect the iframe readystate.