1864168 - Use 1-origin column number for JavaScript locations in console message

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Tooru Fujisawa [:arai]]

SpiderMonkey had been using 0-origin column number for script location, while using 1-origin column number for errors.
bug 1862814 is about to switch to consistently using 1-origin column numbers everywhere.

There are multiple places in Gecko-side that uses 0-origin column number when reporting to console etc.

Comment 1

[Tooru Fujisawa [:arai]]

Attachment: Bug 1864168 - Part 1: Use 1-origin column number in nsIContentSecurityPolicy. r?smaug!

Depends on D193368

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: Bug 1864168 - Part 2: Use 1-origin column number in nsJSUtils::GetCallingLocation. r?smaug!

nsJSUtils::GetCallingLocation is used mostly for console message and logging,
except for the following:

• profiler label in TimeoutHandler
• CC log for TimeoutHandler
• events in nsIConsoleAPIStorage
• JSON used by mozilla::dom::SendReports

Depends on D193369

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: Bug 1864168 - Part 3: Use 1-origin column number for Xray-related console message. r?smaug!

Depends on D193370

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: Bug 1864168 - Part 4: Use 1-origin column number for WebSocket-related console message. r?smaug!

Depends on D193371

Comment 5

[Tooru Fujisawa [:arai]]

Attachment: Bug 1864168 - Part 5: Use 1-origin column number in MOZ_CRASH message in IOUtils. r?smaug!

Depends on D193372

Comment 6

[Tooru Fujisawa [:arai]]

Attachment: Bug 1864168 - Part 6: Use 1-origin column number in ScriptLoader and nsIScriptElement. r?smaug!

Depends on D193373

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

I don't have too strong opinion whether 0 or 1 origin should be used. You may want to ask also for example peterv.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

And this all needs review from devtools peers.

Comment 9

[Tooru Fujisawa [:arai]]

Thank you for reviewing!
I've added devtools-reviewers, and also asked peterv in chat.
the main part of the devtools change is handled in bug 1862693, but yeah, it's better getting their review for things that touches console message.

Comment 10

[Peter Van der Beken [:peterv]]

(In reply to Tooru Fujisawa [:arai] from comment #9)

I've added devtools-reviewers, and also asked peterv in chat.

I think this is fine, hopefully nobody is really relying on this detail. I looked at the Reporting API spec for example, and it doesn't specify whether the line/column numbers are 0-based or 1-based. AFAICT Chrome is using 1-based.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f6646771a26a
Part 1: Use 1-origin column number in nsIContentSecurityPolicy. r=smaug,devtools-reviewers,ochameau,ckerschb
https://hg.mozilla.org/integration/autoland/rev/fa77d852b494
Part 2: Use 1-origin column number in nsJSUtils::GetCallingLocation. r=smaug,anti-tracking-reviewers,devtools-reviewers,ochameau,pbz
https://hg.mozilla.org/integration/autoland/rev/8ffec0d83028
Part 3: Use 1-origin column number for Xray-related console message. r=smaug,devtools-reviewers,ochameau
https://hg.mozilla.org/integration/autoland/rev/d4db9576559b
Part 4: Use 1-origin column number for WebSocket-related console message. r=smaug,devtools-reviewers,ochameau
https://hg.mozilla.org/integration/autoland/rev/2c30c4d757fc
Part 5: Use 1-origin column number in MOZ_CRASH message in IOUtils. r=smaug
https://hg.mozilla.org/integration/autoland/rev/6d4ef1cdfabb
Part 6: Use 1-origin column number in ScriptLoader and nsIScriptElement. r=smaug,devtools-reviewers,hsivonen,ochameau

Comment 12

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/43306 for changes under testing/web-platform/tests

Comment 13

[Natalia Csoregi [:nataliaCs]]

Backed out along with Bug 1862693, Bug 1864155, Bug 1862814, Bug 1865005 for causing bustage on nsRFPService.cpp.

• backout: https://hg.mozilla.org/integration/autoland/rev/275cd8d44829f57197d7d70055f5a88bf890d153
• push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=a4f3e7625abf4496dc121ee0585c349dbcfab0ac
• failure log: https://treeherder.mozilla.org/logviewer?job_id=437233289&repo=autoland&lineNumber=82723

[task 2023-11-22T11:52:46.461Z] 11:52:46     INFO -  In file included from Unified_cpp_resistfingerprinting0.cpp:11:
[task 2023-11-22T11:52:46.461Z] 11:52:46    ERROR -  /builds/worker/checkouts/gecko/toolkit/components/resistfingerprinting/nsRFPService.cpp:1443:26: error: no member named 'zeroOriginValue' in 'JS::ColumnNumberOneOrigin'; did you mean 'oneOriginValue'?
[task 2023-11-22T11:52:46.462Z] 11:52:46     INFO -   1443 |   aColumnNum = columnNum.zeroOriginValue();
[task 2023-11-22T11:52:46.463Z] 11:52:46     INFO -        |                          ^~~~~~~~~~~~~~~
[task 2023-11-22T11:52:46.463Z] 11:52:46     INFO -        |                          oneOriginValue
[task 2023-11-22T11:52:46.464Z] 11:52:46     INFO -  /builds/worker/workspace/obj-build/dist/include/js/ColumnNumber.h:232:12: note: 'oneOriginValue' declared here
[task 2023-11-22T11:52:46.464Z] 11:52:46     INFO -    232 |   uint32_t oneOriginValue() const {
[task 2023-11-22T11:52:46.465Z] 11:52:46     INFO -        |            ^
[task 2023-11-22T11:52:46.465Z] 11:52:46     INFO -  1 error generated.
[task 2023-11-22T11:52:46.465Z] 11:52:46    ERROR -  gmake[4]: *** [/builds/worker/checkouts/gecko/config/rules.mk:690: Unified_cpp_resistfingerprinting0.o] Error 1
[task 2023-11-22T11:52:46.466Z] 11:52:46     INFO -  gmake[4]: Leaving directory '/builds/worker/workspace/obj-build/toolkit/components/resistfingerprinting'
[task 2023-11-22T11:52:46.467Z] 11:52:46     INFO -  gmake[4]: Target 'target-objects' not remade because of errors.
[task 2023-11-22T11:52:46.467Z] 11:52:46    ERROR -  gmake[3]: *** [/builds/worker/checkouts/gecko/config/recurse.mk:72: toolkit/components/resistfingerprinting/target-objects] Error 2

Comment 14

[Tooru Fujisawa [:arai]]

this is an autoland-conflict with bug 1864826 patch
will address it shortly.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/49d94b91efad
Part 1: Use 1-origin column number in nsIContentSecurityPolicy. r=smaug,devtools-reviewers,ochameau,ckerschb
https://hg.mozilla.org/integration/autoland/rev/5420aecaffcd
Part 2: Use 1-origin column number in nsJSUtils::GetCallingLocation. r=smaug,anti-tracking-reviewers,devtools-reviewers,ochameau,pbz
https://hg.mozilla.org/integration/autoland/rev/0fb806d69104
Part 3: Use 1-origin column number for Xray-related console message. r=smaug,devtools-reviewers,ochameau
https://hg.mozilla.org/integration/autoland/rev/2f0ce7338b3f
Part 4: Use 1-origin column number for WebSocket-related console message. r=smaug,devtools-reviewers,ochameau
https://hg.mozilla.org/integration/autoland/rev/a3c471ba10ce
Part 5: Use 1-origin column number in MOZ_CRASH message in IOUtils. r=smaug
https://hg.mozilla.org/integration/autoland/rev/89f0bd90367b
Part 6: Use 1-origin column number in ScriptLoader and nsIScriptElement. r=smaug,devtools-reviewers,hsivonen,ochameau

Comment 16

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 17

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/49d94b91efad
https://hg.mozilla.org/mozilla-central/rev/5420aecaffcd
https://hg.mozilla.org/mozilla-central/rev/0fb806d69104
https://hg.mozilla.org/mozilla-central/rev/2f0ce7338b3f
https://hg.mozilla.org/mozilla-central/rev/a3c471ba10ce
https://hg.mozilla.org/mozilla-central/rev/89f0bd90367b

Comment 18

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot