1849144 - Inline exception doesn't work on the first line of inline scripts

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[Alexandre Poirot [:ochameau]]

Inline preview of exceptions doesn't work when the exception is on the first line of an inline script.

It works for:
data:text/html,<script>%0A[].plop()</script>
But not for:
data:text/html,<script>[].plop()</script>

STR:

• Open the html page in the debugger
• hover the exception

Comment 1

[Alexandre Poirot [:ochameau]]

Note that inline exception are broken in many other ways, which are independant of this first line issue.

• DOM Exceptions aren't highlighted at all:
  data:text/html,<script>%0Adocument.querySelector({})</script>

• Custom exception are highlighted, but can't be previewed:
  data:text/html,<script>%0Athrow new Error("foo")</script>

Comment 2

[Tooru Fujisawa [:arai]]

This is unrelated to SpiderMonkey, but it's a bug in parser-worker.js.

If the file is text/html, parser worker searches for script tag, and generates a source for each tag content.

https://searchfox.org/mozilla-central/rev/19500c006ebf8dc4587eef55357ae26b772391e1/devtools/client/debugger/dist/parser-worker.js#30433,30454,30456,30461,30477-30479

const startScript = /<script[^>]*>/im;
...
  const startMatch = startScript.exec(str);
...
    const startsAt = startMatch.index + startMatch[0].length;
...
      const locIndex = i + startsAt;
...
      return [
        adjustForLineAndColumn(source, {
          index: locIndex,

there, the source's first line has leading whitespace, in order to keep the the column number same:

https://searchfox.org/mozilla-central/rev/19500c006ebf8dc4587eef55357ae26b772391e1/devtools/client/debugger/dist/parser-worker.js#30510-30512,30516

function adjustForLineAndColumn(fullSource, location) {
  const { column, line } = calcLineAndColumn(fullSource, location.index);
  return Object.assign({}, location, {
...
    source: generateWhitespace(column) + location.source,

but the column used there is 1-origin, which means there's one more whitespace than expected:

https://searchfox.org/mozilla-central/rev/19500c006ebf8dc4587eef55357ae26b772391e1/devtools/client/debugger/dist/parser-worker.js#30499,30502

function calcLineAndColumn(source, index) {
...
  const column = lines.pop().length + 1;

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: Bug 1849144 - Fix the number of whitespaces used for preserving the column number of inline script. r?ochameau!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b65a5303a7ae
Fix the number of whitespaces used for preserving the column number of inline script. r=ochameau,devtools-reviewers

Comment 5

[Natalia Csoregi [:nataliaCs]]

Backed out for causing eslint failure.

• backout: https://hg.mozilla.org/integration/autoland/rev/667f94d3510f6398d8cbd1c534f1082bebe26c97
• push: https://treeherder.mozilla.org/jobs?repo=autoland&revision=b65a5303a7aeae06891e819e83d764c52e270f7a
• failure log: https://treeherder.mozilla.org/logviewer?job_id=426657993&repo=autoland&lineNumber=148

[task 2023-08-21T16:57:28.891Z] TEST-UNEXPECTED-WARNING | /builds/worker/checkouts/gecko/devtools/client/debugger/src/components/SecondaryPanes/Frames/Frame.js:39:37 | 'shouldDisplayOriginalLocation' is missing in props validation (react/prop-types)
[task 2023-08-21T16:57:28.891Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/devtools/client/debugger/src/workers/parser/utils/parse-script-tags/customParse.js:0 | This file needs formatting with Prettier (use 'mach lint --fix <path>'). (prettier)
[task 2023-08-21T16:57:28.891Z] TEST-UNEXPECTED-WARNING | /builds/worker/checkouts/gecko/devtools/shared/network-observer/NetworkHelper.sys.mjs:112:22 | Ci.nsIScriptableUnicodeConverter is deprecated. You should use TextEncoder or TextDecoder instead. (mozilla/reject-scriptableunicodeconverter)

note: we do apologise for having to backout this patch for a pretty/prettier eslint, but unfortunately the mach tool that allows us to fix such issues wasn't working for neither of the sheriffs atm

Comment 6

[Tooru Fujisawa [:arai]]

strange. I've run lint fix and there was no error and nothing was fixed.
and actually, the file isn't modified even if I put some wrong indentation.
perhaps it's covered by different configuration or different command?

Comment 7

[Tooru Fujisawa [:arai]]

The file is listed in ThirdPartyPaths.txt, which is not touched by lint --fix.
but apparently it still expects the file passes the linter.

https://searchfox.org/mozilla-central/rev/98d0035da36d786b7ca9191b8a23de9c2c943465/tools/rewriting/ThirdPartyPaths.txt#16

devtools/client/debugger/src/workers/parser/utils/parse-script-tags/

Comment 8

[Tooru Fujisawa [:arai]]

and running lint --fix with the line in ThirdPartyPaths.txt removed modifies the license section from MIT to MPL. which is too bad.
I'll look into why the file is checked by linter in automation.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ccded805beae
Fix the number of whitespaces used for preserving the column number of inline script. r=ochameau,devtools-reviewers

Comment 10

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/ccded805beae