From 2b4618081f0e6becb16260c41a8044a64cc59fe6 Mon Sep 17 00:00:00 2001 From: chenshay Date: Fri, 4 Nov 2016 13:55:58 -0700 Subject: [PATCH 1/4] revert change 4089 --- src/style-installer.js | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/style-installer.js b/src/style-installer.js index 882951664dec..77c7825d3a16 100644 --- a/src/style-installer.js +++ b/src/style-installer.js @@ -17,7 +17,6 @@ import {dev} from './log'; import {documentStateFor} from './document-state'; import {performanceFor} from './performance'; -import {platformFor} from './platform'; import {setStyles} from './style'; import {waitForServices} from './render-delaying-services'; import {resourcesForDoc} from './resources'; @@ -116,17 +115,6 @@ export function makeBodyVisible(doc, opt_waitForServices) { visibility: 'visible', animation: 'none', }); - - // TODO(erwinm, #4097): Remove this when safari technology preview has merged - // the fix for https://github.com/ampproject/amphtml/issues/4047 - // https://bugs.webkit.org/show_bug.cgi?id=159791 which is in r202950. - if (platformFor(doc.defaultView).isSafari()) { - if (doc.body.style['webkitAnimation'] !== undefined) { - doc.body.style['webkitAnimation'] = 'none'; - } else if (doc.body.style['WebkitAnimation'] !== undefined) { - doc.body.style['WebkitAnimation'] = 'none'; - } - } }; /** @const {!Window} */ const win = doc.defaultView; From fa76eb285f8c196f6db626886ff2ce9625f1709f Mon Sep 17 00:00:00 2001 From: chenshay Date: Mon, 21 Nov 2016 18:06:37 -0800 Subject: [PATCH 2/4] set ariane-hidden to visible elements out of view --- extensions/amp-carousel/0.1/slidescroll.js | 6 +++++ .../amp-carousel/0.1/test/test-slidescroll.js | 23 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/extensions/amp-carousel/0.1/slidescroll.js b/extensions/amp-carousel/0.1/slidescroll.js index ec5f8be2047a..cb73772c0436 100644 --- a/extensions/amp-carousel/0.1/slidescroll.js +++ b/extensions/amp-carousel/0.1/slidescroll.js @@ -467,8 +467,12 @@ export class AmpSlideScroll extends BaseSlides { if (showIndex == newIndex) { this.scheduleLayout(this.slides_[showIndex]); this.scheduleResume(this.slides_[showIndex]); + this.slides_[showIndex] && + this.slides_[showIndex].setAttribute('aria-hidden', 'false'); } else { this.schedulePreload(this.slides_[showIndex]); + this.slides_[showIndex] && + this.slides_[showIndex].setAttribute('aria-hidden', 'true'); } }); this.slidesContainer_./*OK*/scrollLeft = @@ -515,6 +519,8 @@ export class AmpSlideScroll extends BaseSlides { setStyle(this.slideWrappers_[i], 'order', ''); } this.slideWrappers_[i].classList.remove(SHOWN_CSS_CLASS); + this.slides_[i] && + this.slides_[i].setAttribute('aria-hidden', 'true'); } // Pause if not the current slide if (this.slideIndex_ != i) { diff --git a/extensions/amp-carousel/0.1/test/test-slidescroll.js b/extensions/amp-carousel/0.1/test/test-slidescroll.js index 21c872d0dcaa..b3cc1849b552 100644 --- a/extensions/amp-carousel/0.1/test/test-slidescroll.js +++ b/extensions/amp-carousel/0.1/test/test-slidescroll.js @@ -91,6 +91,8 @@ describe('SlideScroll', () => { .to.be.true; expect(impl.slideWrappers_[1].classList.contains(SHOW_CLASS)) .to.be.true; + expect(impl.slides_[0].getAttribute('aria-hidden')).to.equal('false'); + expect(impl.slides_[1].getAttribute('aria-hidden')).to.equal('true'); }); }); @@ -192,6 +194,9 @@ describe('SlideScroll', () => { 'amp-carousel-next', {'fromSlide': 'slide-id', 'toSlide': '1'}); expect(analyticsEventSpy).to.have.been.calledWith( 'amp-carousel-change', {'fromSlide': 'slide-id', 'toSlide': '1'}); + expect(impl.slides_[0].getAttribute('aria-hidden')).to.equal('true'); + expect(impl.slides_[1].getAttribute('aria-hidden')).to.equal('false'); + expect(impl.slides_[2].getAttribute('aria-hidden')).to.equal('true'); impl.showSlide_(0); @@ -220,6 +225,8 @@ describe('SlideScroll', () => { 'amp-carousel-prev', {'fromSlide': '1', 'toSlide': 'slide-id'}); expect(analyticsEventSpy).to.have.been.calledWith( 'amp-carousel-change', {'fromSlide': '1', 'toSlide': 'slide-id'}); + expect(impl.slides_[0].getAttribute('aria-hidden')).to.equal('false'); + expect(impl.slides_[1].getAttribute('aria-hidden')).to.equal('true'); impl.showSlide_(4); @@ -246,6 +253,9 @@ describe('SlideScroll', () => { 'amp-carousel-prev', {'fromSlide': 'slide-id', 'toSlide': '4'}); expect(analyticsEventSpy).to.have.been.calledWith( 'amp-carousel-change', {'fromSlide': 'slide-id', 'toSlide': '4'}); + expect(impl.slides_[3].getAttribute('aria-hidden')).to.equal('true'); + expect(impl.slides_[4].getAttribute('aria-hidden')).to.equal('false'); + expect(impl.slides_[0].getAttribute('aria-hidden')).to.equal('true'); }); }); @@ -587,6 +597,10 @@ describe('SlideScroll', () => { sandbox.spy(impl, 'hideRestOfTheSlides_'); const setControlsStateSpy = sandbox.spy(impl, 'setControlsState'); + expect(impl.slides_[4].getAttribute('aria-hidden')).to.equal('true'); + expect(impl.slides_[0].getAttribute('aria-hidden')).to.equal('false'); + expect(impl.slides_[1].getAttribute('aria-hidden')).to.equal('true'); + impl.showSlide_(1); expect(updateInViewportSpy).to.have.been.calledWith( @@ -611,6 +625,9 @@ describe('SlideScroll', () => { expect(hideRestOfTheSlidesSpy).to.have.been.calledWith([0, 1, 2]); expect(hideRestOfTheSlidesSpy.callCount).to.equal(1); expect(setControlsStateSpy.callCount).to.equal(1); + expect(impl.slides_[0].getAttribute('aria-hidden')).to.equal('true'); + expect(impl.slides_[1].getAttribute('aria-hidden')).to.equal('false'); + expect(impl.slides_[2].getAttribute('aria-hidden')).to.equal('true'); impl.showSlide_(0); @@ -637,6 +654,9 @@ describe('SlideScroll', () => { expect(hideRestOfTheSlidesSpy).to.have.been.calledWith([4, 0, 1]); expect(hideRestOfTheSlidesSpy.callCount).to.equal(2); expect(setControlsStateSpy.callCount).to.equal(2); + expect(impl.slides_[4].getAttribute('aria-hidden')).to.equal('true'); + expect(impl.slides_[0].getAttribute('aria-hidden')).to.equal('false'); + expect(impl.slides_[1].getAttribute('aria-hidden')).to.equal('true'); impl.showSlide_(4); @@ -662,6 +682,9 @@ describe('SlideScroll', () => { expect(hideRestOfTheSlidesSpy).to.have.been.calledWith([3, 4, 0]); expect(hideRestOfTheSlidesSpy.callCount).to.equal(3); expect(setControlsStateSpy.callCount).to.equal(3); + expect(impl.slides_[3].getAttribute('aria-hidden')).to.equal('true'); + expect(impl.slides_[4].getAttribute('aria-hidden')).to.equal('false'); + expect(impl.slides_[0].getAttribute('aria-hidden')).to.equal('true'); }); }); From c497f84065ad8945f8c58d7a9532081c42ae6c08 Mon Sep 17 00:00:00 2001 From: chenshay Date: Tue, 22 Nov 2016 09:21:43 -0800 Subject: [PATCH 3/4] remove checks --- extensions/amp-carousel/0.1/slidescroll.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/extensions/amp-carousel/0.1/slidescroll.js b/extensions/amp-carousel/0.1/slidescroll.js index cb73772c0436..f4afcf7043c1 100644 --- a/extensions/amp-carousel/0.1/slidescroll.js +++ b/extensions/amp-carousel/0.1/slidescroll.js @@ -467,12 +467,10 @@ export class AmpSlideScroll extends BaseSlides { if (showIndex == newIndex) { this.scheduleLayout(this.slides_[showIndex]); this.scheduleResume(this.slides_[showIndex]); - this.slides_[showIndex] && - this.slides_[showIndex].setAttribute('aria-hidden', 'false'); + this.slides_[showIndex].setAttribute('aria-hidden', 'false'); } else { this.schedulePreload(this.slides_[showIndex]); - this.slides_[showIndex] && - this.slides_[showIndex].setAttribute('aria-hidden', 'true'); + this.slides_[showIndex].setAttribute('aria-hidden', 'true'); } }); this.slidesContainer_./*OK*/scrollLeft = @@ -519,8 +517,7 @@ export class AmpSlideScroll extends BaseSlides { setStyle(this.slideWrappers_[i], 'order', ''); } this.slideWrappers_[i].classList.remove(SHOWN_CSS_CLASS); - this.slides_[i] && - this.slides_[i].setAttribute('aria-hidden', 'true'); + this.slides_[i].setAttribute('aria-hidden', 'true'); } // Pause if not the current slide if (this.slideIndex_ != i) { From 495c64fd88b3733a8aaf1ece2b51d5cd3acbfb98 Mon Sep 17 00:00:00 2001 From: chenshay Date: Tue, 22 Nov 2016 10:59:45 -0800 Subject: [PATCH 4/4] removing aria-hidden --- extensions/amp-carousel/0.1/slidescroll.js | 2 +- extensions/amp-carousel/0.1/test/test-slidescroll.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/amp-carousel/0.1/slidescroll.js b/extensions/amp-carousel/0.1/slidescroll.js index f4afcf7043c1..f04c239eda1c 100644 --- a/extensions/amp-carousel/0.1/slidescroll.js +++ b/extensions/amp-carousel/0.1/slidescroll.js @@ -517,7 +517,7 @@ export class AmpSlideScroll extends BaseSlides { setStyle(this.slideWrappers_[i], 'order', ''); } this.slideWrappers_[i].classList.remove(SHOWN_CSS_CLASS); - this.slides_[i].setAttribute('aria-hidden', 'true'); + this.slides_[i].removeAttribute('aria-hidden'); } // Pause if not the current slide if (this.slideIndex_ != i) { diff --git a/extensions/amp-carousel/0.1/test/test-slidescroll.js b/extensions/amp-carousel/0.1/test/test-slidescroll.js index fbf31b3cf853..74eaf364e98c 100644 --- a/extensions/amp-carousel/0.1/test/test-slidescroll.js +++ b/extensions/amp-carousel/0.1/test/test-slidescroll.js @@ -255,7 +255,7 @@ describe('SlideScroll', () => { 'amp-carousel-change', {'fromSlide': 'slide-id', 'toSlide': '4'}); expect(impl.slides_[3].getAttribute('aria-hidden')).to.equal('true'); expect(impl.slides_[4].getAttribute('aria-hidden')).to.equal('false'); - expect(impl.slides_[0].getAttribute('aria-hidden')).to.equal('true'); + expect(impl.slides_[0].getAttribute('aria-hidden')).to.equal(null); }); });