Closed Bug 1849144 Opened 10 months ago Closed 9 months ago

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

Categories

(DevTools :: Debugger, defect)

defect

Tracking

(firefox118 fixed)

RESOLVED FIXED
118 Branch
Tracking Status
firefox118 --- fixed

People

(Reporter: ochameau, Assigned: arai)

References

Details

Attachments

(1 file)

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

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>

See Also: → 1845865

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;
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Pushed by arai_a@mac.com:
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

Backed out for causing eslint failure.

[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

Flags: needinfo?(arai.unmht)

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?

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/
Flags: needinfo?(arai.unmht)

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.

Pushed by arai_a@mac.com:
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
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → 118 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: