1224654 - Allow pretty printing and syntax highlight JS files which don't have Content-Type:"text/javascript" and don't end with .js/.jsm

Status: RESOLVED INVALID

(DevTools :: Debugger, defect, P2)

Description

[arni2033]

Attachment: testcase 1 - Pretty printing and syntax highlight don't work on some JS.html

STR:   (Win7_64, Nightly 45, 32bit, ID 20151110030205, new profile)
1. Open attached "testcase 1"
2. Open devtools -> Debugger, click "Pretty print" button  [{}]

Result:
 The syntax isn't highlighted and JavaScript isn't pretty-printed at all. Console says
>   togglePrettyPrint threw an exception: Can't prettify non-javascript files.
 But I can debug such files if they are already properly formatted (with '\n's)

Expectations:
  It should "syntax-highlighted" and pretty-printed normally:
> var Div = document.querySelector('div');
> Div.addEventListener('click', function () {
>   console.log('click event');
> }, false);

Comment 1

[arni2033]

Now here's funny thing: if I append "window.js" in the end of [src] in testcase 1, everything is OK

Comment 2

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I was able to reproduce this bug using this STR:

1. Load http://janodvarko.cz/tests/bugzilla/1224654/
2. Open DevTools Toolbox and select the Debugger panel
3. Expand (no domain) and select the only script there (it's data: URL)
4. There is no way to pretty print, there is not even a status bar with the pretty-print button

Is that because of the data: URL?

Honza

Comment 3

[David Walsh :davidwalsh]

We presently have a function to test if we believe a file is reasonably JavaScript:

export function isJavaScript(source: Source, content: SourceContent): boolean {
  const url = source.url;
  const contentType = content.type === "wasm" ? null : content.contentType;
  return (
    (url && /\.(jsm|js)?$/.test(trimUrlQuery(url))) ||
    !!(contentType && contentType.includes("javascript"))
  );
}

This check is used before allowing pretty-printing a file. Maybe we remove this check and allow pretty-printing everything? Worst case scenario is a silent failure?

Comment 4

[David Walsh :davidwalsh]

This is sill an issue. We could allow pretty-printing everything via:

diff --git a/devtools/client/debugger/src/actions/sources/prettyPrint.js b/devtools/client/debugger/src/actions/sources/prettyPrint.js
--- a/devtools/client/debugger/src/actions/sources/prettyPrint.js
+++ b/devtools/client/debugger/src/actions/sources/prettyPrint.js
@@ -41,9 +41,11 @@ export async function prettyPrintSource(
   content: SourceContent,
   actors: Array<SourceActor>
 ) {
+  /*
   if (!isJavaScript(generatedSource, content) || content.type !== "text") {
     throw new Error("Can't prettify non-javascript files.");
   }
+  */
 
   const url = getPrettySourceURL(generatedSource.url);
   const { code, mappings } = await prettyPrint({
diff --git a/devtools/client/debugger/src/components/Editor/Footer.js b/devtools/client/debugger/src/components/Editor/Footer.js
--- a/devtools/client/debugger/src/components/Editor/Footer.js
+++ b/devtools/client/debugger/src/components/Editor/Footer.js
@@ -281,9 +281,7 @@ const mapStateToProps = state => {
       selectedSource ? selectedSource.id : null
     ),
     endPanelCollapsed: getPaneCollapse(state, "end"),
-    canPrettyPrint: selectedSource
-      ? canPrettyPrintSource(state, selectedSource.id)
-      : false,
+    canPrettyPrint: !!selectedSource,
   };
 };

With that code, I try to pretty-print HTML, I get:

Error loading this URI: Invalid regular expression flag (3:115)

We we need to:

1. Research a library that can handle pretty-printing multiple types of files
2. Decide if these use cases are a priority.

Comment 5

[:Harald Kirschner :digitarald]

It feels this issue throws together 3 issues:

1. Enhancement: Pretty-print inline JS in HTML
2. Pretty-print scripts in data: URL
3. Missing pretty-print button on some obvious JS files (bug 1555307)

Which one would this bug tackle?

Comment 6

[:Harald Kirschner :digitarald]

My proposal: Drop the fine-grained checks and show the pretty-printing button for all files that are not text/html (which we know for sure does not work).

Comment 7

[Zac Svoboda :zacnomore]

Attachment: pretty-print.png

Comment 8

[Zac Svoboda :zacnomore]

Pretty print now works correctly for inline JS