From 37ffbded053b6c861cf38a75023966b2e3fcbb33 Mon Sep 17 00:00:00 2001 From: erwin mombay Date: Tue, 7 Jan 2025 12:51:41 -0800 Subject: [PATCH] Fix: Rename CloseWatcher.signalClose to CloseWatched.requestClose (#40213) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 📦 Update dependency chromedriver to v131 * switch to realWin to resolve scheme-less url to the correct hostname something changed recently with Url Parsing in Chrome or something in the FakeWin implementation is causign it to not resolve properly. Quick fix seems to be to switch it to a real window * try out the disable feature * rename to requestClose * undo skip to test which tests are still failing * add back skip * try w/o closewatcher * Update test/unit/utils/test-close-watcher-impl.js Co-authored-by: Daniel Rozenberg * Update test/unit/utils/test-close-watcher-impl.js Co-authored-by: Daniel Rozenberg * Update test/unit/utils/test-close-watcher-impl.js Co-authored-by: Daniel Rozenberg * Update test/unit/utils/test-close-watcher-impl.js Co-authored-by: Daniel Rozenberg --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Daniel Rozenberg --- .../0.1/test/test-amp-lightbox.js | 2 +- src/utils/close-watcher-impl.js | 8 +- src/utils/close-watcher.extern.js | 2 +- test/unit/utils/test-close-watcher-impl.js | 89 +++++++++++-------- 4 files changed, 56 insertions(+), 45 deletions(-) diff --git a/extensions/amp-lightbox/0.1/test/test-amp-lightbox.js b/extensions/amp-lightbox/0.1/test/test-amp-lightbox.js index f797c77ba44d..0ccafd6c9340 100644 --- a/extensions/amp-lightbox/0.1/test/test-amp-lightbox.js +++ b/extensions/amp-lightbox/0.1/test/test-amp-lightbox.js @@ -114,7 +114,7 @@ describes.realWin( const setupCloseSpy = env.sandbox.spy(impl, 'close'); await impl.open_({caller: sourceElement}); - impl.closeWatcher_.signalClosed(); + impl.closeWatcher_.requestClose(); expect(setupCloseSpy).to.be.called; }); diff --git a/src/utils/close-watcher-impl.js b/src/utils/close-watcher-impl.js index fa525552049c..320f18793daf 100644 --- a/src/utils/close-watcher-impl.js +++ b/src/utils/close-watcher-impl.js @@ -67,11 +67,11 @@ export class CloseWatcherImpl { /** * Signals to the close watcher to close the modal. - * See `CloseWatcher.signalClosed`. + * See `CloseWatcher.requestClose`. */ - signalClosed() { + requestClose() { if (this.watcher_) { - this.watcher_.signalClosed(); + this.watcher_.requestClose(); } else if (this.handler_) { const handler = this.handler_; handler(); @@ -106,7 +106,7 @@ export class CloseWatcherImpl { closeOnEscape_(event) { if (event.key == Keys_Enum.ESCAPE) { event.preventDefault(); - this.signalClosed(); + this.requestClose(); } } } diff --git a/src/utils/close-watcher.extern.js b/src/utils/close-watcher.extern.js index 4a208a563c7c..12ee76aad07f 100644 --- a/src/utils/close-watcher.extern.js +++ b/src/utils/close-watcher.extern.js @@ -12,7 +12,7 @@ function CloseWatcher() {} CloseWatcher.prototype.destroy = function () {}; -CloseWatcher.prototype.signalClosed = function () {}; +CloseWatcher.prototype.requestClose = function () {}; /** @type {?function(!Event)} */ CloseWatcher.prototype.onclose; diff --git a/test/unit/utils/test-close-watcher-impl.js b/test/unit/utils/test-close-watcher-impl.js index 970370eee754..fb1e1067c3aa 100644 --- a/test/unit/utils/test-close-watcher-impl.js +++ b/test/unit/utils/test-close-watcher-impl.js @@ -24,46 +24,57 @@ describes.realWin('#CloseWatcherImpl', {amp: true}, (env) => { historyMock.verify(); }); - // TODO(#40214): fix flaky test. - it.skip('should push and pop history state', async () => { - historyMock.expects('push').resolves('H1').once(); - historyMock.expects('pop').withArgs('H1').once(); - const watcher = new CloseWatcherImpl(ampdoc, handler); - await Promise.resolve('H1'); - watcher.signalClosed(); - expect(handler).to.be.calledOnce; - }); + it.configure() + .if(() => 'CloseWatcher' in window) + .run('should call the handler on requestClose', async () => { + const watcher = new CloseWatcherImpl(ampdoc, handler); + watcher.requestClose(); + expect(handler).to.be.calledOnce; + }); - // TODO(#40214): fix flaky test. - it.skip('should trigger on history pop', async () => { - let popHandler; - historyMock - .expects('push') - .withArgs( - env.sandbox.match((a) => { - popHandler = a; - return true; - }) - ) - .resolves('H1') - .once(); - new CloseWatcherImpl(ampdoc, handler); - await Promise.resolve('H1'); - expect(popHandler).to.exist; - popHandler(); - expect(handler).to.be.calledOnce; - }); + it.configure() + .if(() => !('CloseWatcher' in window)) + .run('should push and pop history state', async () => { + historyMock.expects('push').resolves('H1').once(); + historyMock.expects('pop').withArgs('H1').once(); + const watcher = new CloseWatcherImpl(ampdoc, handler); + await Promise.resolve('H1'); + watcher.requestClose(); + expect(handler).to.be.calledOnce; + }); - // TODO(#40214): fix flaky test. - it.skip('should trigger on ESC key', async () => { - historyMock.expects('push').resolves('H1').once(); - historyMock.expects('pop').withArgs('H1').once(); - new CloseWatcherImpl(ampdoc, handler); - await Promise.resolve('H1'); + it.configure() + .if(() => !('CloseWatcher' in window)) + .run('should trigger on history pop', async () => { + let popHandler; + historyMock + .expects('push') + .withArgs( + env.sandbox.match((a) => { + popHandler = a; + return true; + }) + ) + .resolves('H1') + .once(); + new CloseWatcherImpl(ampdoc, handler); + await Promise.resolve('H1'); + expect(popHandler).to.exist; + popHandler(); + expect(handler).to.be.calledOnce; + }); - doc.documentElement.dispatchEvent( - new KeyboardEvent('keydown', {key: Keys_Enum.ESCAPE}) - ); - expect(handler).to.be.calledOnce; - }); + it.configure() + .if(() => !('CloseWatcher' in window)) + .run('should trigger on ESC key', async () => { + historyMock.expects('push').resolves('H1').once(); + historyMock.expects('pop').withArgs('H1').once(); + new CloseWatcherImpl(ampdoc, handler); + await Promise.resolve('H1'); + + doc.documentElement.dispatchEvent( + new KeyboardEvent('keydown', {key: Keys_Enum.ESCAPE}) + ); + expect(handler).to.be.calledOnce; + }); });