From bd533daa6a28af02e25fbf9c3da4e03a9da6534d Mon Sep 17 00:00:00 2001 From: arya-s Date: Tue, 14 Nov 2023 03:43:00 +0900 Subject: [PATCH 1/2] feat(feedback): Sends MessageEvent when error page embed is closed Sends `reportdialog_closed` via window.postMessage to the parent of the error page embed in preparation of supporting an `onClose` callback in @sentry/browser `showReportDialog`. Fixes https://github.com/getsentry/sentry-javascript/issues/9433 --- .../templates/sentry/error-page-embed.js | 13 ++++- src/sentry/testutils/pytest/selenium.py | 30 ++++++----- tests/acceptance/test_error_page_embed.py | 54 +++++++++++++++++++ 3 files changed, 82 insertions(+), 15 deletions(-) create mode 100644 tests/acceptance/test_error_page_embed.py diff --git a/src/sentry/templates/sentry/error-page-embed.js b/src/sentry/templates/sentry/error-page-embed.js index ae57852f3775dc..0577da6d4588bd 100644 --- a/src/sentry/templates/sentry/error-page-embed.js +++ b/src/sentry/templates/sentry/error-page-embed.js @@ -13,9 +13,11 @@ }; */ + /* eslint-disable spaced-comment */ var strings = /*{{ strings }}*/ ''; var template = /*{{ template }}*/ ''; var endpoint = /*{{ endpoint }}*/ ''; + /* eslint-enable */ var setChild = function (target, child) { target.innerHTML = ''; @@ -66,7 +68,9 @@ this.element.className = 'sentry-error-embed-wrapper'; this.element.innerHTML = template; self.element.onclick = function (e) { - if (e.target !== self.element) return; + if (e.target !== self.element) { + return; + } self.close(); }; @@ -146,11 +150,16 @@ document.removeEventListener('keydown', handleFocus); } this.element.parentNode.removeChild(this.element); + if (window.parent) { + window.parent.postMessage('reportdialog_closed', '*'); + } }; SentryErrorEmbed.prototype.submit = function (body) { var self = this; - if (this._submitInProgress) return; + if (this._submitInProgress) { + return; + } this._submitInProgress = true; var xhr = new XMLHttpRequest(); diff --git a/src/sentry/testutils/pytest/selenium.py b/src/sentry/testutils/pytest/selenium.py index c3f9e7e7fec937..cd0771f61de64e 100644 --- a/src/sentry/testutils/pytest/selenium.py +++ b/src/sentry/testutils/pytest/selenium.py @@ -115,8 +115,8 @@ def set_viewport(self, width, height, fit_content): self.set_window_size(size["previous"]["width"], size["previous"]["height"]) @contextmanager - def full_viewport(self, width=None, height=None): - return self.set_viewport(width, height, fit_content=True) + def full_viewport(self, width=None, height=None, fit_content=True): + return self.set_viewport(width, height, fit_content) @contextmanager def mobile_viewport(self, width=375, height=812): @@ -251,23 +251,27 @@ def wait_until_not(self, selector=None, title=None, timeout=10): return self - def wait_for_images_loaded(self, timeout=10): + def wait_until_script_execution(self, script, timeout=10): + """ + Waits until ``script`` executes and evaluates truthy, + or until ``timeout`` is hit, whichever happens first. + """ wait = WebDriverWait(self.driver, timeout) - wait.until( - lambda driver: driver.execute_script( - """return Object.values(document.querySelectorAll('img')).map(el => el.complete).every(i => i)""" - ) - ) + wait.until(lambda driver: driver.execute_script(script)) return self - def wait_for_fonts_loaded(self, timeout=10): - wait = WebDriverWait(self.driver, timeout) - wait.until( - lambda driver: driver.execute_script("""return document.fonts.status === 'loaded'""") + def wait_for_images_loaded(self, timeout=10): + return self.wait_until_script_execution( + """return Object.values(document.querySelectorAll('img')).map(el => el.complete).every(i => i)""", + timeout, ) - return self + def wait_for_fonts_loaded(self, timeout=10): + return self.wait_until_script_execution( + """return document.fonts.status === 'loaded'""", + timeout, + ) @property def switch_to(self): diff --git a/tests/acceptance/test_error_page_embed.py b/tests/acceptance/test_error_page_embed.py new file mode 100644 index 00000000000000..93b53c1577a480 --- /dev/null +++ b/tests/acceptance/test_error_page_embed.py @@ -0,0 +1,54 @@ +from urllib.parse import quote +from uuid import uuid4 + +from django.urls import reverse + +from sentry.testutils.cases import AcceptanceTestCase +from sentry.testutils.silo import no_silo_test + + +@no_silo_test(stable=True) +class ErrorPageEmbedTest(AcceptanceTestCase): + def setUp(self): + super().setUp() + self.project = self.create_project() + self.key = self.create_project_key(project=self.project) + self.event_id = uuid4().hex + self.path = "{}?eventId={}&dsn={}".format( + reverse("sentry-error-page-embed"), + quote(self.event_id), + quote(self.key.dsn_public), + ) + + def wait_for_error_page_embed(self): + script = f""" + const script = window.document.createElement('script'); + script.async = true; + script.crossOrigin = 'anonymous'; + script.src = '{self.path}'; + const injectionPoint = window.document.head || window.document.body; + injectionPoint.appendChild(script); + + window.addEventListener('message', (event) => {{ + window.__error_page_embed_received_message__ = event.data; + }}); + """ + self.browser.driver.execute_script(script) + self.browser.wait_until(".sentry-error-embed") + + def wait_for_reportdialog_closed_message(self): + self.browser.wait_until_script_execution( + """return window.__error_page_embed_received_message__ === 'reportdialog_closed'""" + ) + + def test_closed_message_received_on_close_button_click(self): + self.wait_for_error_page_embed() + self.browser.click(".sentry-error-embed button.close") + self.wait_for_reportdialog_closed_message() + + def test_closed_message_received_on_outside_click(self): + # in order to click outside we need to set a sufficiently large window size + with self.browser.full_viewport(width=900, height=1330, fit_content=False): + self.wait_for_error_page_embed() + self.browser.click("body") + self.wait_for_reportdialog_closed_message() From 5fffdc0dfd5d3659d4edcf373ff13cd7ae93535f Mon Sep 17 00:00:00 2001 From: arya-s Date: Wed, 22 Nov 2023 18:18:46 +0900 Subject: [PATCH 2/2] feat(feedback): Add `sentry` prefix to reportdialog_closed MessageEvent --- src/sentry/templates/sentry/error-page-embed.js | 2 +- tests/acceptance/test_error_page_embed.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry/templates/sentry/error-page-embed.js b/src/sentry/templates/sentry/error-page-embed.js index 0577da6d4588bd..90b60bb1e249d9 100644 --- a/src/sentry/templates/sentry/error-page-embed.js +++ b/src/sentry/templates/sentry/error-page-embed.js @@ -151,7 +151,7 @@ } this.element.parentNode.removeChild(this.element); if (window.parent) { - window.parent.postMessage('reportdialog_closed', '*'); + window.parent.postMessage('__sentry_reportdialog_closed__', '*'); } }; diff --git a/tests/acceptance/test_error_page_embed.py b/tests/acceptance/test_error_page_embed.py index 93b53c1577a480..d4c80c5f572d4a 100644 --- a/tests/acceptance/test_error_page_embed.py +++ b/tests/acceptance/test_error_page_embed.py @@ -38,7 +38,7 @@ def wait_for_error_page_embed(self): def wait_for_reportdialog_closed_message(self): self.browser.wait_until_script_execution( - """return window.__error_page_embed_received_message__ === 'reportdialog_closed'""" + """return window.__error_page_embed_received_message__ === '__sentry_reportdialog_closed__'""" ) def test_closed_message_received_on_close_button_click(self):