amp-sticky-ad: Force background-color alpha to be 1

extensions/amp-sticky-ad/0.1/amp-sticky-ad.css

@@ -23,7 +23,6 @@ amp-sticky-ad {
   z-index: 11;
   max-height: 100px !important;
   box-sizing: border-box;
-  background-color: rgba(0, 0, 0, 0.5);
 }
 
 .-amp-sticky-ad-layout {

extensions/amp-sticky-ad/0.1/amp-sticky-ad.js

@@ -20,7 +20,14 @@ import {dev,user} from '../../../src/log';
 import {removeElement} from '../../../src/dom';
 import {toggle} from '../../../src/style';
 import {listenOnce} from '../../../src/event-helper';
+import {
+  setStyle,
+  removeAlphaFromColor,
+} from '../../../src/style';
+import {isExperimentOn} from '../../../src/experiments';
 
+/** @private @const {string} */
+const UX_EXPERIMENT = 'amp-sticky-ad-better-ux';
 
 class AmpStickyAd extends AMP.BaseElement {
   /** @param {!AmpElement} element */
@@ -169,6 +176,7 @@ class AmpStickyAd extends AMP.BaseElement {
         // Set sticky-ad to visible and change container style
         this.element.setAttribute('visible', '');
         this.element.classList.add('amp-sticky-ad-loaded');
+        this.forceOpacity_();
       });
     });
   }
@@ -201,6 +209,33 @@ class AmpStickyAd extends AMP.BaseElement {
       this.viewport_.updatePaddingBottom(0);
     });
   }
+
+  /**
+   * To check for background-color alpha and force it to be 1.
+   * Whoever calls this needs to make sure it's in a vsync.
+   * @private
+   */
+  forceOpacity_() {
+    if (!isExperimentOn(this.win, UX_EXPERIMENT)) {
+      return;
+    }
+
+    // TODO(@zhouyx): Move the opacity style to CSS after remove experiments
+    // Note: Use setStyle because we will remove this line later.
+    setStyle(this.element, 'opacity', '1 !important');
+    setStyle(this.element, 'background-image', 'none');
+
+    const backgroundColor = this.win./*OK*/getComputedStyle(this.element)
+        .getPropertyValue('background-color');
+    const newBackgroundColor = removeAlphaFromColor(backgroundColor);
+    if (backgroundColor == newBackgroundColor) {
+      return;
+    }
+
+    user().warn('AMP-STICKY-AD',
+        'Do not allow container to be semitransparent');
+    setStyle(this.element, 'background-color', newBackgroundColor);
+  }
 }
 
 AMP.registerElement('amp-sticky-ad', AmpStickyAd, CSS);

extensions/amp-sticky-ad/0.1/test/test-amp-sticky-ad.js

@@ -15,6 +15,7 @@
  */
 
 import {createIframePromise} from '../../../../testing/iframe';
+import {toggleExperiment} from '../../../../src/experiments';
 import * as sinon from 'sinon';
 import '../amp-sticky-ad';
 import '../../../amp-ad/0.1/amp-ad';
@@ -30,10 +31,13 @@ describe('amp-sticky-ad', () => {
     sandbox.restore();
   });
 
-  function getAmpStickyAd() {
+  function getAmpStickyAd(opt_attributes) {
     return createIframePromise().then(iframe => {
       const ampStickyAd = iframe.doc.createElement('amp-sticky-ad');
       ampStickyAd.setAttribute('layout', 'nodisplay');
+      for (const attr in opt_attributes) {
+        ampStickyAd.setAttribute(attr, opt_attributes[attr]);
+      }
       const ampAd = iframe.doc.createElement('amp-ad');
       ampAd.setAttribute('width', '300');
       ampAd.setAttribute('height', '50');
@@ -377,4 +381,61 @@ describe('amp-sticky-ad', () => {
           .to.be.true;
     });
   });
+
+  it('should not allow container to be set transparent', () => {
+    toggleExperiment(window, 'amp-sticky-ad-better-ux', true);
+    return getAmpStickyAd({
+      'style': 'background-color: rgba(55, 55, 55, 0.55) !important',
+    }).then(obj => {
+      console.log(obj.ampStickyAd);
+      const stickyAdElement = obj.ampStickyAd;
+      const impl = stickyAdElement.implementation_;
+      impl.vsync_.mutate = function(callback) {
+        callback();
+      };
+      impl.scheduleLayoutForAd_();
+      impl.ad_.dispatchEvent(new Event('amp:built'));
+      impl.ad_.dispatchEvent(new Event('amp:load:end'));
+      expect(window.getComputedStyle(stickyAdElement)
+          .getPropertyValue('background-color')).to.equal('rgb(55, 55, 55)');
+    });
+  });
+
+  it('should not allow container to be set semitransparent in rgba', () => {
+    toggleExperiment(window, 'amp-sticky-ad-better-ux', true);
+    return getAmpStickyAd({
+      'style': 'background-color: rgba(55, 55, 55, 0.55) !important',
+    }).then(obj => {
+      console.log(obj.ampStickyAd);
+      const stickyAdElement = obj.ampStickyAd;
+      const impl = stickyAdElement.implementation_;
+      impl.vsync_.mutate = function(callback) {
+        callback();
+      };
+      impl.scheduleLayoutForAd_();
+      impl.ad_.dispatchEvent(new Event('amp:built'));
+      impl.ad_.dispatchEvent(new Event('amp:load:end'));
+      expect(window.getComputedStyle(stickyAdElement)
+          .getPropertyValue('background-color')).to.equal('rgb(55, 55, 55)');
+    });
+  });
+
+  it('should not allow container to be set to transparent', () => {
+    toggleExperiment(window, 'amp-sticky-ad-better-ux', true);
+    return getAmpStickyAd({
+      'style': 'background-color: transparent !important',
+    }).then(obj => {
+      console.log(obj.ampStickyAd);
+      const stickyAdElement = obj.ampStickyAd;
+      const impl = stickyAdElement.implementation_;
+      impl.vsync_.mutate = function(callback) {
+        callback();
+      };
+      impl.scheduleLayoutForAd_();
+      impl.ad_.dispatchEvent(new Event('amp:built'));
+      impl.ad_.dispatchEvent(new Event('amp:load:end'));
+      expect(window.getComputedStyle(stickyAdElement)
+          .getPropertyValue('background-color')).to.equal('rgb(0, 0, 0)');
+    });
+  });
 });

src/style.js

@@ -14,7 +14,6 @@
  * limitations under the License.
  */
 
-
 // Note: loaded by 3p system. Cannot rely on babel polyfills.
 
 /** @type {Object<string, string>} */
@@ -196,3 +195,15 @@ export function translate(x, opt_y) {
 export function scale(value) {
   return `scale(${value})`;
 }
+
+/**
+ * Remove alpha value from a rgba color value.
+ * Return the new color property with alpha equals if has the alpha value.
+ * Caller needs to make sure the input color value is a valid rgba/rgb value
+ * @param {string} rgbaColor
+ * @return {string}
+ */
+export function removeAlphaFromColor(rgbaColor) {
+  return rgbaColor.replace(
+      /\(([^,]+),([^,]+),([^,)]+),[^)]+\)/g, '($1,$2,$3, 1)');
+}

test/functional/test-style.js

@@ -85,6 +85,15 @@ describe('Style', () => {
     expect(st.camelCaseToTitleCase(str)).to.equal('TheQuickBrownFox');
   });
 
+  it('removeAlphaFromColor', () => {
+    expect(st.removeAlphaFromColor('rgba(1, 1, 1, 0)')).to.equal(
+        'rgba(1, 1, 1, 1)');
+    expect(st.removeAlphaFromColor('rgb(1, 1, 1)')).to.equal(
+        'rgb(1, 1, 1)');
+    expect(st.removeAlphaFromColor('rgba(0, 0, 0,-0.5)')).to.equal(
+      'rgba(0, 0, 0, 1)');
+  });
+
   describe('getVendorJsPropertyName', () => {
 
     it('no prefix', () => {

tools/experiments/experiments.js

@@ -218,6 +218,11 @@ const EXPERIMENTS = [
     name: 'Split AMP\'s loading phase into chunks',
     cleanupIssue: 'https://github.com/ampproject/amphtml/issues/5535',
   },
+  {
+    id: 'amp-sticky-ad-better-ux',
+    name: 'New breaking UX changes make to amp-sticky-ad',
+    cleanupIssue: 'https://github.com/ampproject/amphtml/issues/5822',
+  },
 ];
 
 if (getMode().localDev) {