Bug 918797 - Trying to prettify html irrevocably loses the source until the page is refreshed, r=fitzgen

browser/devtools/debugger/debugger-controller.js

@@ -1147,7 +1147,7 @@ SourceScripts.prototype = {
 
   /**
    * Pretty print a source's text. All subsequent calls to |getText| will return
-   * the pretty text.
+   * the pretty text. Nothing will happen for non-javascript files.
    *
    * @param Object aSource
    *        The source form from the RDP.
@@ -1156,8 +1156,13 @@ SourceScripts.prototype = {
    *          [aSource, error].
    */
   prettyPrint: function(aSource) {
-    let textPromise = this._cache.get(aSource.url);
+    // Only attempt to pretty print JavaScript sources.
+    if (!SourceUtils.isJavaScript(aSource.url, aSource.contentType)) {
+      return promise.reject([aSource, "Can't prettify non-javascript files."]);
+    }
+
     // Only use the existing promise if it is pretty printed.
+    let textPromise = this._cache.get(aSource.url);
     if (textPromise && textPromise.pretty) {
       return textPromise;
     }
@@ -1166,12 +1171,17 @@ SourceScripts.prototype = {
     this._cache.set(aSource.url, deferred.promise);
 
     this.activeThread.source(aSource)
-      .prettyPrint(Prefs.editorTabSize, ({ error, message, source }) => {
+      .prettyPrint(Prefs.editorTabSize, ({ error, message, source: text }) => {
         if (error) {
+          // Revert the rejected promise from the cache, so that the original
+          // source's text may be shown when the source is selected.
+          this._cache.set(aSource.url, textPromise);
           deferred.reject([aSource, message || error]);
           return;
         }
 
+        // Remove the cached source AST from the Parser, to avoid getting
+        // wrong locations when searching for functions.
         DebuggerController.Parser.clearSource(aSource.url);
 
         if (this.activeThread.paused) {
@@ -1180,7 +1190,7 @@ SourceScripts.prototype = {
           this.activeThread.fillFrames(CALL_STACK_PAGE_SIZE);
         }
 
-        deferred.resolve([aSource, source]);
+        deferred.resolve([aSource, text]);
       });
 
     deferred.promise.pretty = true;
@@ -1218,14 +1228,14 @@ SourceScripts.prototype = {
     }
 
     // Get the source text from the active thread.
-    this.activeThread.source(aSource).source(aResponse => {
+    this.activeThread.source(aSource).source(({ error, message, source: text }) => {
       if (aOnTimeout) {
         window.clearTimeout(fetchTimeout);
       }
-      if (aResponse.error) {
-        deferred.reject([aSource, aResponse.message || aResponse.error]);
+      if (error) {
+        deferred.reject([aSource, message || error]);
       } else {
-        deferred.resolve([aSource, aResponse.source]);
+        deferred.resolve([aSource, text]);
       }
     });
 

browser/devtools/debugger/debugger-panes.js

@@ -379,19 +379,23 @@ SourcesView.prototype = Heritage.extend(WidgetMethods, {
    * Pretty print the selected source.
    */
   prettyPrint: function() {
-    const resetEditor = () => {
+    const resetEditor = ([{ url }]) => {
       // Only set the text when the source is still selected.
-      if (this.selectedValue === source.url) {
-        DebuggerView.setEditorLocation(source.url, 0, { force: true });
+      if (url == this.selectedValue) {
+        DebuggerView.setEditorLocation(url, 0, { force: true });
       }
     };
+    const printError = ([{ url }, error]) => {
+      let err = DevToolsUtils.safeErrorString(error);
+      let msg = "Couldn't prettify source: " + url + "\n" + err;
+      Cu.reportError(msg);
+      dumpn(msg);
+      return;
+    }
 
     let { source } = this.selectedItem.attachment;
-    // Reset the editor even when we fail, so that we can give the user a clue
-    // as to why the source isn't pretty printed and what happened.
-    DebuggerController.SourceScripts.prettyPrint(source)
-      .then(resetEditor,
-            resetEditor);
+    let prettyPrinted = DebuggerController.SourceScripts.prettyPrint(source);
+    prettyPrinted.then(resetEditor, printError);
   },
 
   /**
@@ -995,6 +999,18 @@ let SourceUtils = {
   _labelsCache: new Map(), // Can't use WeakMaps because keys are strings.
   _groupsCache: new Map(),
 
+  /**
+   * Returns true if the specified url and/or content type are specific to
+   * javascript files.
+   *
+   * @return boolean
+   *         True if the source is likely javascript.
+   */
+  isJavaScript: function(aUrl, aContentType = "") {
+    return /\.jsm?$/.test(this.trimUrlQuery(aUrl)) ||
+           aContentType.contains("javascript");
+  },
+
   /**
    * Clears the labels cache, populated by methods like
    * SourceUtils.getSourceLabel or Source Utils.getSourceGroup.

browser/devtools/debugger/debugger-view.js

@@ -240,26 +240,19 @@ let DebuggerView = {
     // Avoid setting the editor mode for very large files.
     if (aTextContent.length >= SOURCE_SYNTAX_HIGHLIGHT_MAX_FILE_SIZE) {
       this.editor.setMode(SourceEditor.MODES.TEXT);
-      return;
     }
-
-    if (aContentType) {
-      if (/javascript/.test(aContentType)) {
-        this.editor.setMode(SourceEditor.MODES.JAVASCRIPT);
-      } else {
-        this.editor.setMode(SourceEditor.MODES.HTML);
-      }
-    } else if (aTextContent.match(/^\s*</)) {
-      // Use HTML mode for files in which the first non whitespace character is
-      // &lt;, regardless of extension.
+    // Use JS mode for files with .js and .jsm extensions.
+    else if (SourceUtils.isJavaScript(aUrl, aContentType)) {
+      this.editor.setMode(SourceEditor.MODES.JAVASCRIPT);
+    }
+    // Use HTML mode for files in which the first non whitespace character is
+    // &lt;, regardless of extension.
+    else if (aTextContent.match(/^\s*</)) {
       this.editor.setMode(SourceEditor.MODES.HTML);
-    } else {
-      // Use JS mode for files with .js and .jsm extensions.
-      if (/\.jsm?$/.test(SourceUtils.trimUrlQuery(aUrl))) {
-        this.editor.setMode(SourceEditor.MODES.JAVASCRIPT);
-      } else {
-        this.editor.setMode(SourceEditor.MODES.TEXT);
-      }
+    }
+    // Unknown languange, use plain text.
+    else {
+      this.editor.setMode(SourceEditor.MODES.TEXT);
     }
   },
 

browser/devtools/debugger/test/Makefile.in

@@ -54,6 +54,8 @@ MOCHITEST_BROWSER_FILES = \
 	browser_dbg_pretty-print-02.js \
 	browser_dbg_pretty-print-03.js \
 	browser_dbg_pretty-print-04.js \
+	browser_dbg_pretty-print-05.js \
+	browser_dbg_pretty-print-06.js \
 	browser_dbg_progress-listener-bug.js \
 	browser_dbg_reload-preferred-script-01.js \
 	browser_dbg_reload-preferred-script-02.js \
@@ -145,6 +147,7 @@ MOCHITEST_BROWSER_FILES = \
 	doc_minified.html \
 	doc_pause-exceptions.html \
 	doc_pretty-print.html \
+	doc_pretty-print-02.html \
 	doc_recursion-stack.html \
 	doc_script-switching-01.html \
 	doc_script-switching-02.html \
@@ -168,6 +171,7 @@ MOCHITEST_BROWSER_FILES = \
 	code_script-switching-01.js \
 	code_script-switching-02.js \
 	code_test-editor-mode \
+	code_test-syntax-error.js \
 	code_ugly.js \
 	testactors.js \
 	addon1.xpi \

browser/devtools/debugger/test/browser_dbg_pretty-print-05.js

@@ -0,0 +1,78 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Make sure that prettifying HTML sources doesn't do anything.
+ */
+
+const TAB_URL = EXAMPLE_URL + "doc_pretty-print-02.html";
+const JS_URL = EXAMPLE_URL + "code_test-syntax-error.js";
+
+let gTab, gDebuggee, gPanel, gDebugger;
+let gEditor, gSources, gControllerSources;
+
+function test() {
+  // A source with a syntax error will be loaded.
+  ignoreAllUncaughtExceptions();
+
+  initDebugger(TAB_URL).then(([aTab, aDebuggee, aPanel]) => {
+    gTab = aTab;
+    gDebuggee = aDebuggee;
+    gPanel = aPanel;
+    gDebugger = gPanel.panelWin;
+    gEditor = gDebugger.DebuggerView.editor;
+    gSources = gDebugger.DebuggerView.Sources;
+    gControllerSources = gDebugger.DebuggerController.SourceScripts;
+
+    Task.spawn(function() {
+      yield waitForSourceShown(gPanel, TAB_URL);
+
+      // From this point onward, the source editor's text should never change.
+      once(gEditor, SourceEditor.EVENTS.TEXT_CHANGED).then(() => {
+        ok(false, "The source editor text shouldn't have changed.");
+      });
+
+      is(gSources.selectedValue, TAB_URL,
+        "The correct source is currently selected.");
+      ok(gEditor.getText().contains("banana"),
+        "The source shouldn't be pretty printed yet.");
+
+      clickPrettyPrintButton();
+
+      let { source } = gSources.selectedItem.attachment;
+      try {
+        yield gControllerSources.prettyPrint(source);
+        ok(false, "The promise for a prettified source should be rejected!");
+      } catch ([source, error]) {
+        is(error, "Can't prettify non-javascript files.",
+          "The promise was correctly rejected with a meaningful message.");
+      }
+
+      let [source, text] = yield gControllerSources.getText(source);
+      is(gSources.selectedValue, TAB_URL,
+        "The correct source is still selected.");
+      ok(gEditor.getText().contains("banana"),
+        "The displayed source hasn't changed.");
+      ok(text.contains("banana"),
+        "The cached source text wasn't altered in any way.");
+
+      yield closeDebuggerAndFinish(gPanel);
+    });
+  });
+}
+
+function clickPrettyPrintButton() {
+  EventUtils.sendMouseEvent({ type: "click" },
+    gDebugger.document.getElementById("pretty-print"),
+    gDebugger);
+}
+
+registerCleanupFunction(function() {
+  gTab = null;
+  gDebuggee = null;
+  gPanel = null;
+  gDebugger = null;
+  gEditor = null;
+  gSources = null;
+  gControllerSources = null;
+});

browser/devtools/debugger/test/browser_dbg_pretty-print-06.js

@@ -0,0 +1,103 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Make sure that prettifying JS sources with type errors works as expected.
+ */
+
+const TAB_URL = EXAMPLE_URL + "doc_pretty-print-02.html";
+const JS_URL = EXAMPLE_URL + "code_test-syntax-error.js";
+
+let gTab, gDebuggee, gPanel, gDebugger;
+let gEditor, gSources, gControllerSources;
+
+function test() {
+  // A source with a syntax error will be loaded.
+  ignoreAllUncaughtExceptions();
+
+  initDebugger(TAB_URL).then(([aTab, aDebuggee, aPanel]) => {
+    gTab = aTab;
+    gDebuggee = aDebuggee;
+    gPanel = aPanel;
+    gDebugger = gPanel.panelWin;
+    gEditor = gDebugger.DebuggerView.editor;
+    gSources = gDebugger.DebuggerView.Sources;
+    gControllerSources = gDebugger.DebuggerController.SourceScripts;
+
+    Task.spawn(function() {
+      yield waitForSourceShown(gPanel, TAB_URL);
+
+      let reloaded = promise.all([
+        waitForDebuggerEvents(aPanel, gDebugger.EVENTS.NEW_SOURCE),
+        waitForDebuggerEvents(aPanel, gDebugger.EVENTS.SOURCES_ADDED),
+        waitForDebuggerEvents(aPanel, gDebugger.EVENTS.SOURCE_SHOWN)
+      ]);
+
+      is(gSources.itemCount, 1,
+        "There should be 1 item displayed in the sources view before reloading.");
+      is(gSources.values[0], TAB_URL,
+        "The only source shown should be the html page.");
+
+      gDebugger.gClient.activeTab.reload();
+      yield reloaded;
+
+      is(gSources.itemCount, 2,
+        "There should be 2 items displayed in the sources view after reloading.");
+      is(gSources.values[0], JS_URL,
+        "The first source shown should be the js file with the syntax error.");
+      is(gSources.values[1], TAB_URL,
+        "The second source shown should be the html page.");
+
+      let changed = waitForSourceShown(gPanel, JS_URL);
+      gSources.selectedLabel = JS_URL;
+      yield changed;
+
+      // From this point onward, the source editor's text should never change.
+      once(gEditor, SourceEditor.EVENTS.TEXT_CHANGED).then(() => {
+        ok(false, "The source editor text shouldn't have changed.");
+      });
+
+      is(gSources.selectedValue, JS_URL,
+        "The correct source is currently selected.");
+      ok(gEditor.getText().contains("pineapple"),
+        "The source shouldn't be pretty printed yet.");
+
+      clickPrettyPrintButton();
+
+      let { source } = gSources.selectedItem.attachment;
+      try {
+        yield gControllerSources.prettyPrint(source);
+        ok(false, "The promise for a prettified source should be rejected!");
+      } catch ([source, error]) {
+        ok(error.contains("SyntaxError: missing ; before statement"),
+          "The promise was correctly rejected with a SyntaxError message.");
+      }
+
+      let [source, text] = yield gControllerSources.getText(source);
+      is(gSources.selectedValue, JS_URL,
+        "The correct source is still selected.");
+      ok(gEditor.getText().contains("pineapple"),
+        "The displayed source hasn't changed.");
+      ok(text.contains("pineapple"),
+        "The cached source text wasn't altered in any way.");
+
+      yield closeDebuggerAndFinish(gPanel);
+    });
+  });
+}
+
+function clickPrettyPrintButton() {
+  EventUtils.sendMouseEvent({ type: "click" },
+    gDebugger.document.getElementById("pretty-print"),
+    gDebugger);
+}
+
+registerCleanupFunction(function() {
+  gTab = null;
+  gDebuggee = null;
+  gPanel = null;
+  gDebugger = null;
+  gEditor = null;
+  gSources = null;
+  gControllerSources = null;
+});

browser/devtools/debugger/test/code_test-syntax-error.js

@@ -0,0 +1,6 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+function pineapple() {
+  syntax error
+}

browser/devtools/debugger/test/doc_pretty-print-02.html

@@ -0,0 +1,12 @@
+<!-- Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/ -->
+<!DOCTYPE html>
+<head>
+  <meta charset="utf-8"/>
+  <title>Debugger Pretty Printing Test Page</title>
+</head>
+<script src="code_test-syntax-error.js"></script>
+<script type="text/javascript">
+  function banana() {
+  }
+</script>

browser/devtools/debugger/test/doc_pretty-print.html

@@ -1,3 +1,5 @@
+<!-- Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/ -->
 <!DOCTYPE html>
 <head>
   <meta charset="utf-8"/>