Bug 1226548 - Event listeners tooltip breaks the syntax in popup if there's a comment before event handler in JS code r?pbro draft
authorMichael Ratcliffe <mratcliffe@mozilla.com>
Mon, 18 Apr 2016 16:15:56 +0100
changeset 352806 0d24d1e73521ece0a6c4e607f1766da1d919b0a2
parent 352730 fae9c42677310d5dbe380e6e70efff2d46a99cc5
child 518758 e5a17b54da2a21335b8bbbe91f09f1de15eed48f
push id15813
push usermratcliffe@mozilla.com
push dateMon, 18 Apr 2016 21:35:25 +0000
reviewerspbro
bugs1226548
milestone48.0a1
Bug 1226548 - Event listeners tooltip breaks the syntax in popup if there's a comment before event handler in JS code r?pbro MozReview-Commit-ID: 2CXAMDhICxn
devtools/client/inspector/markup/test/browser_markup_events.js
devtools/client/inspector/markup/test/browser_markup_events_jquery_1.0.js
devtools/client/inspector/markup/test/browser_markup_events_jquery_1.1.js
devtools/client/inspector/markup/test/doc_markup_events.html
devtools/server/actors/inspector.js
--- a/devtools/client/inspector/markup/test/browser_markup_events.js
+++ b/devtools/client/inspector/markup/test/browser_markup_events.js
@@ -27,17 +27,17 @@ const TEST_DATA = [ // eslint-disable-li
       }
     ]
   },
   {
     selector: "#container",
     expected: [
       {
         type: "mouseover",
-        filename: TEST_URL + ":62",
+        filename: TEST_URL + ":72",
         attributes: [
           "Capturing",
           "DOM2"
         ],
         handler: "function mouseoverHandler(event) {\n" +
                  "  if (event.target.id !== \"container\") {\n" +
                  "    let output = document.getElementById(\"output\");\n" +
                  "    output.textContent = event.target.textContent;\n" +
@@ -46,29 +46,29 @@ const TEST_DATA = [ // eslint-disable-li
       }
     ]
   },
   {
     selector: "#multiple",
     expected: [
       {
         type: "click",
-        filename: TEST_URL + ":69",
+        filename: TEST_URL + ":79",
         attributes: [
           "Bubbling",
           "DOM2"
         ],
         handler: "function clickHandler(event) {\n" +
                  "  let output = document.getElementById(\"output\");\n" +
                  "  output.textContent = \"click\";\n" +
                  "}"
       },
       {
         type: "mouseup",
-        filename: TEST_URL + ":78",
+        filename: TEST_URL + ":88",
         attributes: [
           "Bubbling",
           "DOM2"
         ],
         handler: "function mouseupHandler(event) {\n" +
                  "  let output = document.getElementById(\"output\");\n" +
                  "  output.textContent = \"mouseup\";\n" +
                  "}"
@@ -89,65 +89,65 @@ const TEST_DATA = [ // eslint-disable-li
       }
     ]
   },
   {
     selector: "#handleevent",
     expected: [
       {
         type: "click",
-        filename: TEST_URL + ":89",
+        filename: TEST_URL + ":99",
         attributes: [
           "Bubbling",
           "DOM2"
         ],
         handler: "handleEvent: function(blah) {\n" +
                  "  alert(\"handleEvent clicked\");\n" +
                  "}"
       }
     ]
   },
   {
     selector: "#fatarrow",
     expected: [
       {
         type: "click",
-        filename: TEST_URL + ":57",
+        filename: TEST_URL + ":59",
         attributes: [
           "Bubbling",
           "DOM2"
         ],
         handler: "event => {\n" +
                  "  alert(\"Yay for the fat arrow!\");\n" +
                  "}"
       }
     ]
   },
   {
     selector: "#boundhe",
     expected: [
       {
         type: "click",
-        filename: TEST_URL + ":101",
+        filename: TEST_URL + ":111",
         attributes: [
           "Bubbling",
           "DOM2"
         ],
         handler: "handleEvent: function() {\n" +
                  "  alert(\"boundHandleEvent clicked\");\n" +
                  "}"
       }
     ]
   },
   {
     selector: "#bound",
     expected: [
       {
         type: "click",
-        filename: TEST_URL + ":74",
+        filename: TEST_URL + ":84",
         attributes: [
           "Bubbling",
           "DOM2"
         ],
         handler: "function boundClickHandler(event) {\n" +
                  "  alert(\"Bound event clicked\");\n" +
                  "}"
       }
@@ -164,17 +164,17 @@ const TEST_DATA = [ // eslint-disable-li
     beforeTest: function* (inspector, testActor) {
       let nodeMutated = inspector.once("markupmutation");
       yield testActor.eval("window.wrappedJSObject.addNoeventsClickHandler();");
       yield nodeMutated;
     },
     expected: [
       {
         type: "click",
-        filename: TEST_URL + ":106",
+        filename: TEST_URL + ":116",
         attributes: [
           "Bubbling",
           "DOM2"
         ],
         handler: "function noeventsClickHandler(event) {\n" +
                  "  alert(\"noevents has an event listener\");\n" +
                  "}"
       }
@@ -185,13 +185,45 @@ const TEST_DATA = [ // eslint-disable-li
     beforeTest: function* (inspector, testActor) {
       let nodeMutated = inspector.once("markupmutation");
       yield testActor.eval(
         "window.wrappedJSObject.removeNoeventsClickHandler();");
       yield nodeMutated;
     },
     expected: []
   },
+  {
+    selector: "#comment-inline",
+    expected: [
+      {
+        type: "click",
+        filename: TEST_URL + ":131",
+        attributes: [
+          "Bubbling",
+          "DOM2"
+        ],
+        handler: "function functionProceededByInlineComment() {\n" +
+                 "  alert(\"comment-inline has an event listener\");\n" +
+                 "}"
+      }
+    ]
+  },
+  {
+    selector: "#comment-streaming",
+    expected: [
+      {
+        type: "click",
+        filename: TEST_URL + ":136",
+        attributes: [
+          "Bubbling",
+          "DOM2"
+        ],
+        handler: "function functionProceededByStreamingComment() {\n" +
+                 "  alert(\"comment-streaming has an event listener\");\n" +
+                 "}"
+      }
+    ]
+  },
 ];
 
 add_task(function* () {
   yield runEventPopupTests(TEST_URL, TEST_DATA);
 });
--- a/devtools/client/inspector/markup/test/browser_markup_events_jquery_1.0.js
+++ b/devtools/client/inspector/markup/test/browser_markup_events_jquery_1.0.js
@@ -18,18 +18,17 @@ const TEST_DATA = [
     selector: "html",
     expected: [
       {
         type: "load",
         filename: URL_ROOT + TEST_LIB,
         attributes: [
           "jQuery"
         ],
-        handler: "// Handle when the DOM is ready\n" +
-                 "ready: function() {\n" +
+        handler: "ready: function() {\n" +
                  "  // Make sure that the DOM is not already loaded\n" +
                  "  if (!jQuery.isReady) {\n" +
                  "    // Remember that the DOM is ready\n" +
                  "    jQuery.isReady = true;\n" +
                  "\n" +
                  "    // If there are functions bound, to execute\n" +
                  "    if (jQuery.readyList) {\n" +
                  "      // Execute all of them\n" +
--- a/devtools/client/inspector/markup/test/browser_markup_events_jquery_1.1.js
+++ b/devtools/client/inspector/markup/test/browser_markup_events_jquery_1.1.js
@@ -18,18 +18,17 @@ const TEST_DATA = [
     selector: "html",
     expected: [
       {
         type: "load",
         filename: URL_ROOT + TEST_LIB,
         attributes: [
           "jQuery"
         ],
-        handler: "// Handle when the DOM is ready\n" +
-                 "ready: function() {\n" +
+        handler: "ready: function() {\n" +
                  "  // Make sure that the DOM is not already loaded\n" +
                  "  if (!jQuery.isReady) {\n" +
                  "    // Remember that the DOM is ready\n" +
                  "    jQuery.isReady = true;\n" +
                  "\n" +
                  "    // If there are functions bound, to execute\n" +
                  "    if (jQuery.readyList) {\n" +
                  "      // Execute all of them\n" +
--- a/devtools/client/inspector/markup/test/doc_markup_events.html
+++ b/devtools/client/inspector/markup/test/doc_markup_events.html
@@ -16,17 +16,19 @@
     }
 
     #output,
     #noevents,
     #DOM0,
     #handleevent,
     #fatarrow,
     #bound,
-    #boundhe {
+    #boundhe,
+    #comment-inline,
+    #comment-streaming {
       border: 1px solid #000;
       width: 200px;
       min-height: 1em;
       cursor: pointer;
     }
 
     #output,
     #noevents {
@@ -52,16 +54,24 @@
 
         let bound = document.getElementById("bound");
         boundClickHandler = boundClickHandler.bind(this);
         bound.addEventListener("click", boundClickHandler);
 
         fatarrow.addEventListener("click", event => {
           alert("Yay for the fat arrow!");
         });
+
+        let commentInline = document.getElementById("comment-inline");
+        commentInline.addEventListener("click",
+                                       functionProceededByInlineComment);
+
+        let commentStreaming = document.getElementById("comment-streaming");
+        commentStreaming.addEventListener("click",
+                                          functionProceededByStreamingComment);
       }
 
       function mouseoverHandler(event) {
         if (event.target.id !== "container") {
           let output = document.getElementById("output");
           output.textContent = event.target.textContent;
         }
       }
@@ -100,27 +110,37 @@
       boundHandleEventClick.prototype = {
         handleEvent: function() {
           alert("boundHandleEvent clicked");
         }
       };
 
       function noeventsClickHandler(event) {
         alert("noevents has an event listener");
-      };
+      }
 
       function addNoeventsClickHandler() {
         let noevents = document.getElementById("noevents");
         noevents.addEventListener("click", noeventsClickHandler);
-      };
+      }
 
       function removeNoeventsClickHandler() {
         let noevents = document.getElementById("noevents");
         noevents.removeEventListener("click", noeventsClickHandler);
-      };
+      }
+
+      // A function proceeded with an inline comment
+      function functionProceededByInlineComment() {
+        alert("comment-inline has an event listener");
+      }
+
+      /* A function proceeded with a streaming comment */
+      function functionProceededByStreamingComment() {
+        alert("comment-streaming has an event listener");
+      }
     </script>
   </head>
   <body onload="init();">
     <div id="container">
       <div>1</div>
       <div>2</div>
       <div>3</div>
       <div>4</div>
@@ -140,10 +160,12 @@
     </div>
     <div id="output"></div>
     <div id="noevents">No events here</div>
     <div id="DOM0" onclick="alert('hi')">DOM0 event here</div>
     <div id="handleevent">handleEvent event here</div>
     <div id="fatarrow">Fat arrow event</div>
     <div id="boundhe">Bound handleEvent</div>
     <div id="bound">Bound event</div>
+    <div id="comment-inline">Event proceeded by an inline comment</div>
+    <div id="comment-streaming">Event proceeded by a streaming comment</div>
   </body>
 </html>
--- a/devtools/server/actors/inspector.js
+++ b/devtools/server/actors/inspector.js
@@ -558,23 +558,26 @@ var NodeActor = exports.NodeActor = prot
 
     So we need to work back to the preceeding \n, ; or } so we can get the
       appropriate function info e.g.:
       () => { doSomething(); }
       function doit() { doSomething(); }
       doit: function() { doSomething(); }
     */
     let scriptBeforeFunc = scriptSource.substr(0, script.sourceStart);
+
     let lastEnding = Math.max(
       scriptBeforeFunc.lastIndexOf(";"),
       scriptBeforeFunc.lastIndexOf("}"),
       scriptBeforeFunc.lastIndexOf("{"),
       scriptBeforeFunc.lastIndexOf("("),
       scriptBeforeFunc.lastIndexOf(","),
-      scriptBeforeFunc.lastIndexOf("!")
+      scriptBeforeFunc.lastIndexOf("!"),
+      scriptBeforeFunc.lastIndexOf("/"),
+      scriptBeforeFunc.lastIndexOf("\n")
     );
 
     if (lastEnding !== -1) {
       let functionPrefix = scriptBeforeFunc.substr(lastEnding + 1);
       functionSource = functionPrefix + functionSource;
     }
 
     let dom0 = false;