From 43bd0cf40a4304df8017a170ebfb0e6956f82258 Mon Sep 17 00:00:00 2001
From: zhouyx <zhouyx@google.com>
Date: Tue, 25 Oct 2016 11:50:39 -0700
Subject: [PATCH 1/9] check background-color alpha

---
 .../amp-sticky-ad/0.1/amp-sticky-ad.css       |  2 +-
 extensions/amp-sticky-ad/0.1/amp-sticky-ad.js | 26 +++++++++++++++++++
 .../0.1/test/test-amp-sticky-ad.js            | 25 +++++++++++++++++-
 tools/experiments/experiments.js              |  5 ++++
 4 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/extensions/amp-sticky-ad/0.1/amp-sticky-ad.css b/extensions/amp-sticky-ad/0.1/amp-sticky-ad.css
index 65bf558a9adf..2bc0344a3ab7 100644
--- a/extensions/amp-sticky-ad/0.1/amp-sticky-ad.css
+++ b/extensions/amp-sticky-ad/0.1/amp-sticky-ad.css
@@ -23,7 +23,7 @@ amp-sticky-ad {
   z-index: 11;
   max-height: 100px !important;
   box-sizing: border-box;
-  background-color: rgba(0, 0, 0, 0.5);
+  background-color: #fff;
 }
 
 .-amp-sticky-ad-layout {
diff --git a/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js b/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
index de555d240914..e5432ef8ce9f 100644
--- a/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
+++ b/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
@@ -20,7 +20,12 @@ import {dev,user} from '../../../src/log';
 import {removeElement} from '../../../src/dom';
 import {toggle} from '../../../src/style';
 import {listenOnce} from '../../../src/event-helper';
+import {startsWith} from '../../../src/string';
+import {setStyle} from '../../../src/style';
+import {isExperimentOn} from '../../../src/experiments';
 
+/** @private @const {string} */
+const TAG = 'amp-sticky-ad-v1';
 
 class AmpStickyAd extends AMP.BaseElement {
   /** @param {!AmpElement} element */
@@ -169,6 +174,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 +207,26 @@ class AmpStickyAd extends AMP.BaseElement {
       this.viewport_.updatePaddingBottom(0);
     });
   }
+
+  // Whoever call this needs to make sure it's in a vsync.
+  forceOpacity_() {
+    if (!isExperimentOn(this.win, TAG)) {
+      return;
+    }
+    const background = this.win.getComputedStyle(this.element)
+        .getPropertyValue('background-color');
+    if (!startsWith(background, 'rgba')) {
+      return;
+    }
+    user().warn('AMP-STICKY-AD', 'Do not allow container to be transparent');
+    const backgroundColor = background.substring(
+        background.indexOf('(') + 1,
+        background.lastIndexOf(','));
+    console.log('set styles');
+    setStyle(this.element, 'background-color', 'rgb(' + backgroundColor + ')');
+  }
 }
 
+
+
 AMP.registerElement('amp-sticky-ad', AmpStickyAd, CSS);
diff --git a/extensions/amp-sticky-ad/0.1/test/test-amp-sticky-ad.js b/extensions/amp-sticky-ad/0.1/test/test-amp-sticky-ad.js
index 113c80296d07..c167f162837f 100644
--- a/extensions/amp-sticky-ad/0.1/test/test-amp-sticky-ad.js
+++ b/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,23 @@ describe('amp-sticky-ad', () => {
           .to.be.true;
     });
   });
+
+  it('should not allow container to be set transparent', () => {
+    toggleExperiment(window, 'amp-sticky-ad-v1', 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)');
+    });
+  });
 });
diff --git a/tools/experiments/experiments.js b/tools/experiments/experiments.js
index d07d99ff64d4..c6758005e986 100644
--- a/tools/experiments/experiments.js
+++ b/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-v1',
+    name: 'New breaking UX changes make to amp-sticky-ad',
+    cleanupIssue: 'https://github.com/ampproject/amphtml/issues/5432',
+  },
 ];
 
 if (getMode().localDev) {

From 7d1073f4c863c20768dcab4f8afec52223a855d9 Mon Sep 17 00:00:00 2001
From: zhouyx <zhouyx@google.com>
Date: Tue, 25 Oct 2016 16:40:00 -0700
Subject: [PATCH 2/9] s

---
 extensions/amp-sticky-ad/0.1/amp-sticky-ad.js | 2 --
 1 file changed, 2 deletions(-)

diff --git a/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js b/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
index e5432ef8ce9f..4e1fbb63e372 100644
--- a/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
+++ b/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
@@ -227,6 +227,4 @@ class AmpStickyAd extends AMP.BaseElement {
   }
 }
 
-
-
 AMP.registerElement('amp-sticky-ad', AmpStickyAd, CSS);

From caabd2ae85f1e2ed066b427ce8ec9d45ebbb8f0f Mon Sep 17 00:00:00 2001
From: zhouyx <zhouyx@google.com>
Date: Tue, 25 Oct 2016 16:41:27 -0700
Subject: [PATCH 3/9] update JSdoc

---
 extensions/amp-sticky-ad/0.1/amp-sticky-ad.js | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js b/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
index 4e1fbb63e372..aa979e828503 100644
--- a/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
+++ b/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
@@ -208,7 +208,11 @@ class AmpStickyAd extends AMP.BaseElement {
     });
   }
 
-  // Whoever call this needs to make sure it's in a vsync.
+  /**
+   * To check for background-color alpha and force it to be 1.
+   * Whoever call this needs to make sure it's in a vsync.
+   * @private
+   */
   forceOpacity_() {
     if (!isExperimentOn(this.win, TAG)) {
       return;

From 3cf0c967fff8cfa32c24a12ac15cc722c26c466d Mon Sep 17 00:00:00 2001
From: zhouyx <zhouyx@google.com>
Date: Tue, 25 Oct 2016 17:59:42 -0700
Subject: [PATCH 4/9] address comment 1

---
 extensions/amp-sticky-ad/0.1/amp-sticky-ad.css |  1 -
 extensions/amp-sticky-ad/0.1/amp-sticky-ad.js  | 16 +++++++++++-----
 tools/experiments/experiments.js               |  4 ++--
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/extensions/amp-sticky-ad/0.1/amp-sticky-ad.css b/extensions/amp-sticky-ad/0.1/amp-sticky-ad.css
index 2bc0344a3ab7..674864345170 100644
--- a/extensions/amp-sticky-ad/0.1/amp-sticky-ad.css
+++ b/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: #fff;
 }
 
 .-amp-sticky-ad-layout {
diff --git a/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js b/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
index aa979e828503..2029b9f91426 100644
--- a/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
+++ b/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
@@ -25,7 +25,7 @@ import {setStyle} from '../../../src/style';
 import {isExperimentOn} from '../../../src/experiments';
 
 /** @private @const {string} */
-const TAG = 'amp-sticky-ad-v1';
+const UX_EXPERIMENT = 'amp-sticky-ad-better-ux';
 
 class AmpStickyAd extends AMP.BaseElement {
   /** @param {!AmpElement} element */
@@ -98,6 +98,7 @@ class AmpStickyAd extends AMP.BaseElement {
 
   /** @override */
   collapsedCallback() {
+    console.log('collapsed Callback being called');
     toggle(this.element, false);
     this.vsync_.mutate(() => {
       this.viewport_.updatePaddingBottom(0);
@@ -167,9 +168,11 @@ class AmpStickyAd extends AMP.BaseElement {
    * @private
    */
   layoutAd_() {
+    console.log('layout ad');
     this.updateInViewport(dev().assertElement(this.ad_), true);
     this.scheduleLayout(dev().assertElement(this.ad_));
     listenOnce(this.ad_, 'amp:load:end', () => {
+      console.log('ad loaded');
       this.vsync_.mutate(() => {
         // Set sticky-ad to visible and change container style
         this.element.setAttribute('visible', '');
@@ -210,14 +213,14 @@ class AmpStickyAd extends AMP.BaseElement {
 
   /**
    * To check for background-color alpha and force it to be 1.
-   * Whoever call this needs to make sure it's in a vsync.
+   * Whoever calls this needs to make sure it's in a vsync.
    * @private
    */
   forceOpacity_() {
-    if (!isExperimentOn(this.win, TAG)) {
+    if (!isExperimentOn(this.win, UX_EXPERIMENT)) {
       return;
     }
-    const background = this.win.getComputedStyle(this.element)
+    const background = this.win./*OK*/getComputedStyle(this.element)
         .getPropertyValue('background-color');
     if (!startsWith(background, 'rgba')) {
       return;
@@ -226,7 +229,10 @@ class AmpStickyAd extends AMP.BaseElement {
     const backgroundColor = background.substring(
         background.indexOf('(') + 1,
         background.lastIndexOf(','));
-    console.log('set styles');
+
+    // TODO(@zhouyx): Move the opacity style to CSS after remove experiments
+    // Not using setStyles because we will remove this line later.
+    setStyle(this.element, 'opacity', '1 !important');
     setStyle(this.element, 'background-color', 'rgb(' + backgroundColor + ')');
   }
 }
diff --git a/tools/experiments/experiments.js b/tools/experiments/experiments.js
index c6758005e986..5c58e753d715 100644
--- a/tools/experiments/experiments.js
+++ b/tools/experiments/experiments.js
@@ -219,9 +219,9 @@ const EXPERIMENTS = [
     cleanupIssue: 'https://github.com/ampproject/amphtml/issues/5535',
   },
   {
-    id: 'amp-sticky-ad-v1',
+    id: 'amp-sticky-ad-better-ux',
     name: 'New breaking UX changes make to amp-sticky-ad',
-    cleanupIssue: 'https://github.com/ampproject/amphtml/issues/5432',
+    cleanupIssue: 'https://github.com/ampproject/amphtml/issues/5822',
   },
 ];
 

From 07261fe8d05edb756d5269de75b64d4b7594d876 Mon Sep 17 00:00:00 2001
From: zhouyx <zhouyx@google.com>
Date: Tue, 25 Oct 2016 18:57:16 -0700
Subject: [PATCH 5/9] refactor func to style.js

---
 extensions/amp-sticky-ad/0.1/amp-sticky-ad.js | 29 +++++++++----------
 src/style.js                                  | 22 +++++++++++++-
 test/functional/test-style.js                 | 11 +++++++
 3 files changed, 46 insertions(+), 16 deletions(-)

diff --git a/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js b/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
index 2029b9f91426..87afbdebcd03 100644
--- a/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
+++ b/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
@@ -20,8 +20,10 @@ import {dev,user} from '../../../src/log';
 import {removeElement} from '../../../src/dom';
 import {toggle} from '../../../src/style';
 import {listenOnce} from '../../../src/event-helper';
-import {startsWith} from '../../../src/string';
-import {setStyle} from '../../../src/style';
+import {
+  setStyle,
+  removeAlphaFromBackgroundColor,
+} from '../../../src/style';
 import {isExperimentOn} from '../../../src/experiments';
 
 /** @private @const {string} */
@@ -98,7 +100,6 @@ class AmpStickyAd extends AMP.BaseElement {
 
   /** @override */
   collapsedCallback() {
-    console.log('collapsed Callback being called');
     toggle(this.element, false);
     this.vsync_.mutate(() => {
       this.viewport_.updatePaddingBottom(0);
@@ -168,11 +169,9 @@ class AmpStickyAd extends AMP.BaseElement {
    * @private
    */
   layoutAd_() {
-    console.log('layout ad');
     this.updateInViewport(dev().assertElement(this.ad_), true);
     this.scheduleLayout(dev().assertElement(this.ad_));
     listenOnce(this.ad_, 'amp:load:end', () => {
-      console.log('ad loaded');
       this.vsync_.mutate(() => {
         // Set sticky-ad to visible and change container style
         this.element.setAttribute('visible', '');
@@ -220,20 +219,20 @@ class AmpStickyAd extends AMP.BaseElement {
     if (!isExperimentOn(this.win, UX_EXPERIMENT)) {
       return;
     }
-    const background = this.win./*OK*/getComputedStyle(this.element)
-        .getPropertyValue('background-color');
-    if (!startsWith(background, 'rgba')) {
-      return;
-    }
-    user().warn('AMP-STICKY-AD', 'Do not allow container to be transparent');
-    const backgroundColor = background.substring(
-        background.indexOf('(') + 1,
-        background.lastIndexOf(','));
 
     // TODO(@zhouyx): Move the opacity style to CSS after remove experiments
     // Not using setStyles because we will remove this line later.
     setStyle(this.element, 'opacity', '1 !important');
-    setStyle(this.element, 'background-color', 'rgb(' + backgroundColor + ')');
+
+    const backgroundColor = this.win./*OK*/getComputedStyle(this.element)
+        .getPropertyValue('background-color');
+    const newBackgroundColor = removeAlphaFromBackgroundColor(backgroundColor);
+    if (backgroundColor == newBackgroundColor) {
+      return;
+    }
+
+    user().warn('AMP-STICKY-AD', 'Do not allow container to be transparent');
+    setStyle(this.element, 'background-color', newBackgroundColor);
   }
 }
 
diff --git a/src/style.js b/src/style.js
index eee76d5aa7a3..359cd030c125 100644
--- a/src/style.js
+++ b/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,24 @@ export function translate(x, opt_y) {
 export function scale(value) {
   return `scale(${value})`;
 }
+
+/**
+ * Remove alpha value from a "background-color" CSS property.
+ * Return the new "background-color" property without alpha.
+ * Note caller needs to make sure the input cssColorValue is a valid
+ * CSS "background-color" value.
+ * @param{string} cssColorValue
+ * @return {?string}
+ */
+export function removeAlphaFromBackgroundColor(cssColorValue) {
+  let res;
+  const sepIndex = cssColorValue.indexOf('(');
+  const method = cssColorValue.substring(0, sepIndex);
+  if (method == 'rgba' || method == 'hsla') {
+    const value = cssColorValue.substring(
+        sepIndex,
+        cssColorValue.lastIndexOf(','));
+    res = method.substring(0, 3) + value + ')';
+  }
+  return res || cssColorValue;
+}
diff --git a/test/functional/test-style.js b/test/functional/test-style.js
index e0dedb3effa0..28e491742267 100644
--- a/test/functional/test-style.js
+++ b/test/functional/test-style.js
@@ -85,6 +85,17 @@ describe('Style', () => {
     expect(st.camelCaseToTitleCase(str)).to.equal('TheQuickBrownFox');
   });
 
+  it('removeAlphaFromBackgroundColor', () => {
+    expect(st.removeAlphaFromBackgroundColor('rgba(1, 1, 1, 0)')).to.equal(
+        'rgb(1, 1, 1)');
+    expect(st.removeAlphaFromBackgroundColor('hsla(1, 1, 1, 0.6)')).to.equal(
+        'hsl(1, 1, 1)');
+    expect(st.removeAlphaFromBackgroundColor('rgb(1, 1, 1)')).to.equal(
+        'rgb(1, 1, 1)');
+    expect(st.removeAlphaFromBackgroundColor('hsla(1, 1, 1, 0.6)')).to.equal(
+        'hsl(1, 1, 1)');
+  });
+
   describe('getVendorJsPropertyName', () => {
 
     it('no prefix', () => {

From 638a8593a9195307feb09f8312945d47743a28e7 Mon Sep 17 00:00:00 2001
From: zhouyx <zhouyx@google.com>
Date: Wed, 26 Oct 2016 11:01:24 -0700
Subject: [PATCH 6/9] fix test

---
 extensions/amp-sticky-ad/0.1/test/test-amp-sticky-ad.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/extensions/amp-sticky-ad/0.1/test/test-amp-sticky-ad.js b/extensions/amp-sticky-ad/0.1/test/test-amp-sticky-ad.js
index c167f162837f..ac9df73500a4 100644
--- a/extensions/amp-sticky-ad/0.1/test/test-amp-sticky-ad.js
+++ b/extensions/amp-sticky-ad/0.1/test/test-amp-sticky-ad.js
@@ -383,7 +383,7 @@ describe('amp-sticky-ad', () => {
   });
 
   it('should not allow container to be set transparent', () => {
-    toggleExperiment(window, 'amp-sticky-ad-v1', true);
+    toggleExperiment(window, 'amp-sticky-ad-better-ux', true);
     return getAmpStickyAd({
       'style': 'background-color: rgba(55, 55, 55, 0.55) !important',
     }).then(obj => {

From e3dd99d894d33bfdfb74a0d534d9847fc86bd37a Mon Sep 17 00:00:00 2001
From: zhouyx <zhouyx@google.com>
Date: Wed, 26 Oct 2016 14:36:10 -0700
Subject: [PATCH 7/9] apply regular expression

---
 extensions/amp-sticky-ad/0.1/amp-sticky-ad.js |  4 ++--
 src/style.js                                  | 20 ++++++-------------
 test/functional/test-style.js                 | 14 ++++++-------
 3 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js b/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
index 87afbdebcd03..f907f02ee3da 100644
--- a/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
+++ b/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
@@ -22,7 +22,7 @@ import {toggle} from '../../../src/style';
 import {listenOnce} from '../../../src/event-helper';
 import {
   setStyle,
-  removeAlphaFromBackgroundColor,
+  removeAlphaFromColor,
 } from '../../../src/style';
 import {isExperimentOn} from '../../../src/experiments';
 
@@ -226,7 +226,7 @@ class AmpStickyAd extends AMP.BaseElement {
 
     const backgroundColor = this.win./*OK*/getComputedStyle(this.element)
         .getPropertyValue('background-color');
-    const newBackgroundColor = removeAlphaFromBackgroundColor(backgroundColor);
+    const newBackgroundColor = removeAlphaFromColor(backgroundColor);
     if (backgroundColor == newBackgroundColor) {
       return;
     }
diff --git a/src/style.js b/src/style.js
index 359cd030c125..ab7c899439da 100644
--- a/src/style.js
+++ b/src/style.js
@@ -198,21 +198,13 @@ export function scale(value) {
 
 /**
  * Remove alpha value from a "background-color" CSS property.
- * Return the new "background-color" property without alpha.
+ * Return the new "background-color" property with alpha equals 1.
  * Note caller needs to make sure the input cssColorValue is a valid
  * CSS "background-color" value.
- * @param{string} cssColorValue
- * @return {?string}
+ * @param {string} cssColor
+ * @return {string}
  */
-export function removeAlphaFromBackgroundColor(cssColorValue) {
-  let res;
-  const sepIndex = cssColorValue.indexOf('(');
-  const method = cssColorValue.substring(0, sepIndex);
-  if (method == 'rgba' || method == 'hsla') {
-    const value = cssColorValue.substring(
-        sepIndex,
-        cssColorValue.lastIndexOf(','));
-    res = method.substring(0, 3) + value + ')';
-  }
-  return res || cssColorValue;
+export function removeAlphaFromColor(cssColor) {
+  return cssColor.replace(
+      /\(([^,]+),([^,]+),([^,)]+),[^)]+\)/g, '($1,$2,$3, 1)');
 }
diff --git a/test/functional/test-style.js b/test/functional/test-style.js
index 28e491742267..0e3cfcb915ce 100644
--- a/test/functional/test-style.js
+++ b/test/functional/test-style.js
@@ -85,14 +85,14 @@ describe('Style', () => {
     expect(st.camelCaseToTitleCase(str)).to.equal('TheQuickBrownFox');
   });
 
-  it('removeAlphaFromBackgroundColor', () => {
-    expect(st.removeAlphaFromBackgroundColor('rgba(1, 1, 1, 0)')).to.equal(
+  it('removeAlphaFromColor', () => {
+    expect(st.removeAlphaFromColor('rgba(1, 1, 1, 0)')).to.equal(
+        'rgba(1, 1, 1, 1)');
+    expect(st.removeAlphaFromColor('hsla(1, 1, 1, 0.6)')).to.equal(
+        'hsla(1, 1, 1, 1)');
+    expect(st.removeAlphaFromColor('rgb(1, 1, 1)')).to.equal(
         'rgb(1, 1, 1)');
-    expect(st.removeAlphaFromBackgroundColor('hsla(1, 1, 1, 0.6)')).to.equal(
-        'hsl(1, 1, 1)');
-    expect(st.removeAlphaFromBackgroundColor('rgb(1, 1, 1)')).to.equal(
-        'rgb(1, 1, 1)');
-    expect(st.removeAlphaFromBackgroundColor('hsla(1, 1, 1, 0.6)')).to.equal(
+    expect(st.removeAlphaFromColor('hsl(1, 1, 1)')).to.equal(
         'hsl(1, 1, 1)');
   });
 

From fe854542546f796165af82d3f72df282c2b6a4a1 Mon Sep 17 00:00:00 2001
From: zhouyx <zhouyx@google.com>
Date: Wed, 26 Oct 2016 16:27:25 -0700
Subject: [PATCH 8/9] address comment

---
 extensions/amp-sticky-ad/0.1/amp-sticky-ad.js |  5 +++--
 src/style.js                                  |  6 +++---
 test/functional/test-style.js                 | 12 ++++++++----
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js b/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
index f907f02ee3da..2d5bb7358c7e 100644
--- a/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
+++ b/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
@@ -221,7 +221,7 @@ class AmpStickyAd extends AMP.BaseElement {
     }
 
     // TODO(@zhouyx): Move the opacity style to CSS after remove experiments
-    // Not using setStyles because we will remove this line later.
+    // Note: Use setStyle because we will remove this line later.
     setStyle(this.element, 'opacity', '1 !important');
 
     const backgroundColor = this.win./*OK*/getComputedStyle(this.element)
@@ -231,7 +231,8 @@ class AmpStickyAd extends AMP.BaseElement {
       return;
     }
 
-    user().warn('AMP-STICKY-AD', 'Do not allow container to be transparent');
+    user().warn('AMP-STICKY-AD',
+        'Do not allow container to be semitransparent');
     setStyle(this.element, 'background-color', newBackgroundColor);
   }
 }
diff --git a/src/style.js b/src/style.js
index ab7c899439da..d77a31acabb4 100644
--- a/src/style.js
+++ b/src/style.js
@@ -197,10 +197,10 @@ export function scale(value) {
 }
 
 /**
- * Remove alpha value from a "background-color" CSS property.
- * Return the new "background-color" property with alpha equals 1.
+ * Remove alpha value from a color CSS property.
+ * Return the new color property with alpha equals 1.
  * Note caller needs to make sure the input cssColorValue is a valid
- * CSS "background-color" value.
+ * CSS color value.
  * @param {string} cssColor
  * @return {string}
  */
diff --git a/test/functional/test-style.js b/test/functional/test-style.js
index 0e3cfcb915ce..15877ab04231 100644
--- a/test/functional/test-style.js
+++ b/test/functional/test-style.js
@@ -88,12 +88,16 @@ describe('Style', () => {
   it('removeAlphaFromColor', () => {
     expect(st.removeAlphaFromColor('rgba(1, 1, 1, 0)')).to.equal(
         'rgba(1, 1, 1, 1)');
-    expect(st.removeAlphaFromColor('hsla(1, 1, 1, 0.6)')).to.equal(
-        'hsla(1, 1, 1, 1)');
+    expect(st.removeAlphaFromColor('hsla(120, 100%, 25%, 0.6)')).to.equal(
+        'hsla(120, 100%, 25%, 1)');
     expect(st.removeAlphaFromColor('rgb(1, 1, 1)')).to.equal(
         'rgb(1, 1, 1)');
-    expect(st.removeAlphaFromColor('hsl(1, 1, 1)')).to.equal(
-        'hsl(1, 1, 1)');
+    expect(st.removeAlphaFromColor('hsl(120, 100%, 25%)')).to.equal(
+        'hsl(120, 100%, 25%)');
+    expect(st.removeAlphaFromColor('hsla(120, 100%, 25%, -0.5)')).to.equal(
+        'hsla(120, 100%, 25%, 1)');
+    expect(st.removeAlphaFromColor('white')).to.equal('white');
+    expect(st.removeAlphaFromColor('')).to.equal('');
   });
 
   describe('getVendorJsPropertyName', () => {

From 8b8aab5fbcbb180d546fc4030192427d60f85ef0 Mon Sep 17 00:00:00 2001
From: zhouyx <zhouyx@google.com>
Date: Wed, 26 Oct 2016 17:05:58 -0700
Subject: [PATCH 9/9] add new test

---
 extensions/amp-sticky-ad/0.1/amp-sticky-ad.js |  1 +
 .../0.1/test/test-amp-sticky-ad.js            | 38 +++++++++++++++++++
 src/style.js                                  | 13 +++----
 test/functional/test-style.js                 | 10 +----
 4 files changed, 47 insertions(+), 15 deletions(-)

diff --git a/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js b/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
index 2d5bb7358c7e..d0dc7957fac4 100644
--- a/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
+++ b/extensions/amp-sticky-ad/0.1/amp-sticky-ad.js
@@ -223,6 +223,7 @@ class AmpStickyAd extends AMP.BaseElement {
     // 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');
diff --git a/extensions/amp-sticky-ad/0.1/test/test-amp-sticky-ad.js b/extensions/amp-sticky-ad/0.1/test/test-amp-sticky-ad.js
index ac9df73500a4..d12b28e3573a 100644
--- a/extensions/amp-sticky-ad/0.1/test/test-amp-sticky-ad.js
+++ b/extensions/amp-sticky-ad/0.1/test/test-amp-sticky-ad.js
@@ -400,4 +400,42 @@ describe('amp-sticky-ad', () => {
           .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)');
+    });
+  });
 });
diff --git a/src/style.js b/src/style.js
index d77a31acabb4..4bcb72638aab 100644
--- a/src/style.js
+++ b/src/style.js
@@ -197,14 +197,13 @@ export function scale(value) {
 }
 
 /**
- * Remove alpha value from a color CSS property.
- * Return the new color property with alpha equals 1.
- * Note caller needs to make sure the input cssColorValue is a valid
- * CSS color value.
- * @param {string} cssColor
+ * 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(cssColor) {
-  return cssColor.replace(
+export function removeAlphaFromColor(rgbaColor) {
+  return rgbaColor.replace(
       /\(([^,]+),([^,]+),([^,)]+),[^)]+\)/g, '($1,$2,$3, 1)');
 }
diff --git a/test/functional/test-style.js b/test/functional/test-style.js
index 15877ab04231..09b234fd3e10 100644
--- a/test/functional/test-style.js
+++ b/test/functional/test-style.js
@@ -88,16 +88,10 @@ describe('Style', () => {
   it('removeAlphaFromColor', () => {
     expect(st.removeAlphaFromColor('rgba(1, 1, 1, 0)')).to.equal(
         'rgba(1, 1, 1, 1)');
-    expect(st.removeAlphaFromColor('hsla(120, 100%, 25%, 0.6)')).to.equal(
-        'hsla(120, 100%, 25%, 1)');
     expect(st.removeAlphaFromColor('rgb(1, 1, 1)')).to.equal(
         'rgb(1, 1, 1)');
-    expect(st.removeAlphaFromColor('hsl(120, 100%, 25%)')).to.equal(
-        'hsl(120, 100%, 25%)');
-    expect(st.removeAlphaFromColor('hsla(120, 100%, 25%, -0.5)')).to.equal(
-        'hsla(120, 100%, 25%, 1)');
-    expect(st.removeAlphaFromColor('white')).to.equal('white');
-    expect(st.removeAlphaFromColor('')).to.equal('');
+    expect(st.removeAlphaFromColor('rgba(0, 0, 0,-0.5)')).to.equal(
+      'rgba(0, 0, 0, 1)');
   });
 
   describe('getVendorJsPropertyName', () => {