Просмотр исходного кода

fix: Cleanup toolbar iframe views (#78406)

Quick pass through to remove extra markup returned inside the iframe
views that sentry-toolbar loads.

Also, I moved the `setTimeout` call up inside the if-statement, so that
its behavior aligns with the text on the page: if the window closes on
it's own then everything is ok. If the window does not close then you
want to read the page, and probably checkout the console. Right now the
window closes on it's own no matter what so no debugging info is
available.

- don't need a `style` attr inside a `<link>`
- todo not needed, we can log on the server
- removed the `</body>` tag because A) the browser will figure it out,
B) [`ViewAsRenderMiddleware`
](https://github.com/getsentry/getsentry/blob/4138852300adce937b1cbea9d727b2391ee8aef1/getsentry/vendor/viewas/middleware.py#L102-L132)
was injecting some extra html into there, which wasn't breaking anything
afaik, but it also was just adding weight to a iframe that no one will
see.
- Also, i added a conditional for the logging stuff. It can be noisy to
have logging on by default, but nice to have when we need it.
Ryan Albrecht 5 месяцев назад
Родитель
Сommit
991da06dcf

+ 6 - 4
src/sentry/templates/sentry/toolbar/iframe-invalid.html

@@ -10,7 +10,7 @@ Required context variables:
 <html lang="en">
   <head>
     <title>Invalid Request - Sentry DevToolbar iFrame</title>
-    <link rel="icon" type="image/png" href="{% absolute_asset_url "sentry" "images/favicon.png" %}" style="color:red">
+    <link rel="icon" type="image/png" href="{% absolute_asset_url "sentry" "images/favicon.png" %}">
   </head>
   <body>
     {% script %}
@@ -18,7 +18,6 @@ Required context variables:
       const referrer = "{{ referrer|escapejs }}";
 
       {% if not has_project %}
-      // TODO: use logger and/or analytics
       console.log('Project does not exist.');
       window.parent.postMessage({
         source: 'sentry-toolbar',
@@ -35,5 +34,8 @@ Required context variables:
       {% endif %}
     </script>
     {% endscript %}
-  </body>
-</html>
+
+{% comment %}
+No need to close `body`. If we do then middleware will inject some extra markup
+we don't need. Browsers can figure out when it missing and deal with it.
+{% endcomment %}

+ 8 - 3
src/sentry/templates/sentry/toolbar/iframe.html

@@ -17,7 +17,9 @@ Required context variables:
       const referrer = "{{ referrer|escapejs }}";
 
       function log(...args) {
-        console.log('iframe:', ...args);
+        if (localStorage && localStorage.getItem('sentry-toolbar-iframe-debug')) {
+          console.log('iframe:', ...args);
+        }
       }
 
       log('Init', {referrer});
@@ -65,5 +67,8 @@ Required context variables:
       log('Sent', {message: 'port-connect', referrer})
     </script>
     {% endscript %}
-  </body>
-</html>
+
+{% comment %}
+No need to close `body`. If we do then middleware will inject some extra markup
+we don't need. Browsers can figure out when it missing and deal with it.
+{% endcomment %}

+ 10 - 17
src/sentry/templates/sentry/toolbar/login-success.html

@@ -15,28 +15,21 @@
 
     {% script %}
     <script>
-      (function() {
-        function notifyOpener() {
-          if (!window.opener) {
-            return;
-          }
+      // If the popup doesn't close, we can let people try to close it manually.
+      document.getElementById('close-popup').addEventListener('click', () => {
+        window.close();
+      });
 
-          window.opener.postMessage({
-            source: 'sentry-toolbar',
-            message: 'did-login',
-          }, '*');
-        }
+      if (window.opener) {
+        window.opener.postMessage({
+          source: 'sentry-toolbar',
+          message: 'did-login',
+        }, '*');
 
-        notifyOpener();
         setTimeout(() => {
           window.close();
         }, 3_000);
-
-        // If the popup doesn't close, we can let people try to close it manually.
-        document.getElementById('close-popup').addEventListener('click', () => {
-          window.close();
-        });
-      })();
+      }
     </script>
     {% endscript %}
   </body>