GoogleChromeLabs / chromium-bidi

Implementation of WebDriver BiDi for Chromium
https://googlechromelabs.github.io/chromium-bidi/
Apache License 2.0
90 stars 31 forks source link

Minified mapper contains non-ascii characters, which fails chromium presubmits when embedding it #858

Closed thiagowfx closed 1 year ago

thiagowfx commented 1 year ago

How to test:

$ LC_ALL=C tr -d '[:print:]' < lib/cjs/bidiTab/bidiTab.js | tr -d '\n'
◂▸%

It causes the following issue:

Step compile (with patch) failed. Error logs are shown below:
[62/1772] ACTION //chrome/test/chromedriver:embed_bidimapper_in_cpp(//build/toolchain/win:win_clang_x64)
FAILED: gen/chrome/test/chromedriver/bidimapper/bidimapper.cc gen/chrome/test/chromedriver/bidimapper/bidimapper.h
C:/b/s/w/ir/cipd_bin_packages/cpython3/bin/python3.exe ../../chrome/test/chromedriver/embed_bidimapper_in_cpp.py --directory gen/chrome/test/chromedriver/bidimapper gen/third_party/bidimapper/mapper.js
Traceback (most recent call last):
File "../../chrome/test/chromedriver/embed_bidimapper_in_cpp.py", line 43, in <module>
sys.exit(main())
File "../../chrome/test/chromedriver/embed_bidimapper_in_cpp.py", line 38, in main
...The message was too long...
thiagowfx commented 1 year ago

cc @mathiasbynens @Lightning00Blade @sadym-chromium FYI

thiagowfx commented 1 year ago

I tried ascii_only:

    terser({
      format: {
        ascii_only: true,
      },
    }),

...in rollup.config.mjs but it makes no difference.

I also tried:

    terser({
      output: {
        ascii_only: true,
      },
    }),

...which also makes no difference. output is deprecated though.

Will investigate other approaches.

thiagowfx commented 1 year ago

https://github.com/mbostock/rollup-plugin-ascii also does not work:

% npm run clean; npm run build

> chromium-bidi@0.4.12 clean
> rimraf lib .eslintcache .wireit

> chromium-bidi@0.4.12 build
> wireit

🏃 [tsc] Running command "tsc --build src/tsconfig.json --pretty"
✅ [tsc] Executed successfully
🏃 [rollup] Running command "rollup -c"

lib/cjs/bidiTab/bidiTab.js → lib/iife/mapperTab.js...
[!] (plugin commonjs--resolver) SyntaxError: Unexpected token (47:14) in /Users/tperrotta/Projects/chromium-bidi/lib/cjs/protocol/protocol.js
lib/cjs/bidiTab/bidiTab.js (47:14)
    at Parser.pp$4.raise (/Users/tperrotta/Projects/chromium-bidi/node_modules/rollup-plugin-ascii/node_modules/acorn/dist/acorn.js:2221:15)
    at Parser.pp.unexpected (/Users/tperrotta/Projects/chromium-bidi/node_modules/rollup-plugin-ascii/node_modules/acorn/dist/acorn.js:603:10)
    at Parser.pp.expect (/Users/tperrotta/Projects/chromium-bidi/node_modules/rollup-plugin-ascii/node_modules/acorn/dist/acorn.js:597:28)
    at Parser.pp$3.parseMethod (/Users/tperrotta/Projects/chromium-bidi/node_modules/rollup-plugin-ascii/node_modules/acorn/dist/acorn.js:2071:10)
    at Parser.pp$1.parseClassMethod (/Users/tperrotta/Projects/chromium-bidi/node_modules/rollup-plugin-ascii/node_modules/acorn/dist/acorn.js:1137:25)
    at Parser.pp$1.parseClass (/Users/tperrotta/Projects/chromium-bidi/node_modules/rollup-plugin-ascii/node_modules/acorn/dist/acorn.js:1118:14)
    at Parser.pp$1.parseStatement (/Users/tperrotta/Projects/chromium-bidi/node_modules/rollup-plugin-ascii/node_modules/acorn/dist/acorn.js:697:19)
    at Parser.pp$1.parseBlock (/Users/tperrotta/Projects/chromium-bidi/node_modules/rollup-plugin-ascii/node_modules/acorn/dist/acorn.js:981:25)
    at Parser.pp$3.parseFunctionBody (/Users/tperrotta/Projects/chromium-bidi/node_modules/rollup-plugin-ascii/node_modules/acorn/dist/acorn.js:2105:24)
    at Parser.pp$1.parseFunction (/Users/tperrotta/Projects/chromium-bidi/node_modules/rollup-plugin-ascii/node_modules/acorn/dist/acorn.js:1065:10)
mathiasbynens commented 1 year ago

I tried ascii_only:

    terser({
      format: {
        ascii_only: true,
      },
    }),

...in rollup.config.mjs but it makes no difference.

Weird, it seems to work for me. After git checkout 9335b4b (i.e. before the change replacing non-ASCII characters with ASCII ones), running npm run build without any Rollup config changes:

mathiasb at mathbook in ~/projects/chromium-bidi on 9335b4b [!]
$ npm run build; npx is-ascii-safe-cli lib/iife/mapperTab.js

> chromium-bidi@0.4.12 build
> wireit

✅ [tsc] Already fresh
🏃 [rollup] Running command "rollup -c"

lib/cjs/bidiTab/bidiTab.js → lib/iife/mapperTab.js...
created lib/iife/mapperTab.js in 1.2s
✅ [rollup] Executed successfully
✅ [build] No command to execute
~projects/chromium-bidi/lib/iife/mapperTab.js is not ASCII-safe!
- Found non-ASCII symbol  at position 82342
- Found non-ASCII symbol  at position 82371
- Found non-ASCII symbol  at position 82394
- Found non-ASCII symbol  at position 82415
- Found non-ASCII symbol  at position 82441
- Found non-ASCII symbol  at position 82461
- Found non-ASCII symbol  at position 82483
- Found non-ASCII symbol  at position 82506
- Found non-ASCII symbol  at position 82528
- Found non-ASCII symbol  at position 82536
- Found non-ASCII symbol  at position 82558
- Found non-ASCII symbol  at position 82566
- Found non-ASCII symbol  at position 82590
- Found non-ASCII symbol  at position 82598
- Found non-ASCII symbol  at position 82618
- Found non-ASCII symbol  at position 82640
- Found non-ASCII symbol  at position 82663
- Found non-ASCII symbol  at position 82681
- Found non-ASCII symbol  at position 82689
- Found non-ASCII symbol  at position 82712
- Found non-ASCII symbol  at position 82720
- Found non-ASCII symbol  at position 82745
- Found non-ASCII symbol  at position 82753
- Found non-ASCII symbol  at position 82773
- Found non-ASCII symbol  at position 82781
- Found non-ASCII symbol  at position 82802
- Found non-ASCII symbol  at position 82810
- Found non-ASCII symbol  at position 82836
- Found non-ASCII symbol  at position 82844
- Found non-ASCII symbol  at position 82868
- Found non-ASCII symbol  at position 82876
- Found non-ASCII symbol  at position 82903
- Found non-ASCII symbol  at position 82911
- Found non-ASCII symbol  at position 82937
- Found non-ASCII symbol  at position 82945
- Found non-ASCII symbol  at position 82968
- Found non-ASCII symbol  at position 82976
- Found non-ASCII symbol  at position 82999
- Found non-ASCII symbol  at position 83017
- Found non-ASCII symbol  at position 83035
- Found non-ASCII symbol  at position 83053
- Found non-ASCII symbol  at position 83071
- Found non-ASCII symbol  at position 83089
- Found non-ASCII symbol  at position 83107
- Found non-ASCII symbol  at position 83125
- Found non-ASCII symbol  at position 83143
- Found non-ASCII symbol  at position 83161
- Found non-ASCII symbol  at position 83179
- Found non-ASCII symbol  at position 83197
- Found non-ASCII symbol  at position 83215
- Found non-ASCII symbol  at position 83233
- Found non-ASCII symbol  at position 83251
- Found non-ASCII symbol  at position 83269
- Found non-ASCII symbol  at position 83287
- Found non-ASCII symbol  at position 83305
- Found non-ASCII symbol  at position 83323
- Found non-ASCII symbol  at position 83342
- Found non-ASCII symbol  at position 83361
- Found non-ASCII symbol  at position 83380
- Found non-ASCII symbol  at position 83399
- Found non-ASCII symbol  at position 83418
- Found non-ASCII symbol  at position 83437
- Found non-ASCII symbol  at position 83456
- Found non-ASCII symbol  at position 83475
- Found non-ASCII symbol  at position 83494
- Found non-ASCII symbol  at position 83514
- Found non-ASCII symbol  at position 83534
- Found non-ASCII symbol  at position 83554
- Found non-ASCII symbol  at position 83562
- Found non-ASCII symbol  at position 83583
- Found non-ASCII symbol  at position 83737
- Found non-ASCII symbol  at position 85107
- Found non-ASCII symbol  at position 85131
- Found non-ASCII symbol  at position 85156
- Found non-ASCII symbol  at position 85184
- Found non-ASCII symbol  at position 85213
- Found non-ASCII symbol  at position 85235
- Found non-ASCII symbol  at position 85260
- Found non-ASCII symbol  at position 85286
- Found non-ASCII symbol  at position 85312
- Found non-ASCII symbol  at position 85347
- Found non-ASCII symbol  at position 85369
- Found non-ASCII symbol  at position 85389
- Found non-ASCII symbol  at position 85412
- Found non-ASCII symbol  at position 85432
- Found non-ASCII symbol  at position 85453
- Found non-ASCII symbol  at position 85474
- Found non-ASCII symbol  at position 85497
- Found non-ASCII symbol  at position 85522
- Found non-ASCII symbol  at position 85545
- Found non-ASCII symbol  at position 85571
- Found non-ASCII symbol  at position 85597
- Found non-ASCII symbol  at position 85624
- Found non-ASCII symbol  at position 85648
- Found non-ASCII symbol  at position 85671
- Found non-ASCII symbol  at position 85690
- Found non-ASCII symbol  at position 85709
- Found non-ASCII symbol  at position 85728
- Found non-ASCII symbol  at position 85747
- Found non-ASCII symbol  at position 85766
- Found non-ASCII symbol  at position 85785
- Found non-ASCII symbol  at position 85804
- Found non-ASCII symbol  at position 85823
- Found non-ASCII symbol  at position 85842
- Found non-ASCII symbol  at position 85862
- Found non-ASCII symbol  at position 85882
- Found non-ASCII symbol  at position 85902
- Found non-ASCII symbol  at position 85910
- Found non-ASCII symbol  at position 85934
- Found non-ASCII symbol  at position 85942
- Found non-ASCII symbol  at position 85966
- Found non-ASCII symbol  at position 85974
- Found non-ASCII symbol  at position 85998
- Found non-ASCII symbol  at position 86006
- Found non-ASCII symbol  at position 86030
- Found non-ASCII symbol  at position 86038
- Found non-ASCII symbol  at position 86062
- Found non-ASCII symbol  at position 86086
- Found non-ASCII symbol  at position 86094
- Found non-ASCII symbol  at position 86118
- Found non-ASCII symbol  at position 86126
- Found non-ASCII symbol  at position 86150
- Found non-ASCII symbol  at position 86158
- Found non-ASCII symbol  at position 86182
- Found non-ASCII symbol  at position 86190
- Found non-ASCII symbol  at position 86214
- Found non-ASCII symbol  at position 86240
- Found non-ASCII symbol  at position 86268
- Found non-ASCII symbol  at position 86276
- Found non-ASCII symbol  at position 86306
- Found non-ASCII symbol  at position 86335
- Found non-ASCII symbol  at position 86363
- Found non-ASCII symbol  at position 86394
- Found non-ASCII symbol  at position 86481
- Found non-ASCII symbol  at position 86489
- Found non-ASCII symbol  at position 86497
- Found non-ASCII symbol  at position 86505
- Found non-ASCII symbol  at position 86513
- Found non-ASCII symbol  at position 86530
- Found non-ASCII symbol  at position 86538
- Found non-ASCII symbol  at position 86546
- Found non-ASCII symbol  at position 86554
- Found non-ASCII symbol  at position 86562
- Found non-ASCII symbol  at position 86570
- Found non-ASCII symbol  at position 86578
- Found non-ASCII symbol  at position 86586
- Found non-ASCII symbol  at position 86594
- Found non-ASCII symbol  at position 86602
- Found non-ASCII symbol  at position 86610
- Found non-ASCII symbol  at position 86618
- Found non-ASCII symbol  at position 86626
- Found non-ASCII symbol  at position 86634
- Found non-ASCII symbol  at position 86642
- Found non-ASCII symbol  at position 86650
- Found non-ASCII symbol  at position 86658
- Found non-ASCII symbol  at position 86666
- Found non-ASCII symbol  at position 86674
- Found non-ASCII symbol  at position 86682
- Found non-ASCII symbol  at position 86690
- Found non-ASCII symbol  at position 86698
- Found non-ASCII symbol  at position 86706
- Found non-ASCII symbol  at position 86714
- Found non-ASCII symbol  at position 86722
- Found non-ASCII symbol  at position 86730
- Found non-ASCII symbol  at position 86747
- Found non-ASCII symbol  at position 86755
- Found non-ASCII symbol  at position 86763
- Found non-ASCII symbol  at position 86771
- Found non-ASCII symbol ▸ at position 145767
- Found non-ASCII symbol ◂ at position 145879
- Found non-ASCII symbol ◂ at position 149923
- Found non-ASCII symbol ▸ at position 150181

Then, after applying this change:

diff --git a/rollup.config.mjs b/rollup.config.mjs
index 438d022..659318b 100644
--- a/rollup.config.mjs
+++ b/rollup.config.mjs
@@ -33,6 +33,10 @@ export default {
       // without webcrypto exposes globally.
       ignore: ['crypto'],
     }),
-    terser(),
+    terser({
+      format: {
+        ascii_only: true,
+      },
+    }),
   ],
 };

Then building + checking again:

$ npm run build; npx is-ascii-safe-cli lib/iife/mapperTab.js

> chromium-bidi@0.4.12 build
> wireit

✅ [tsc] Already fresh
✅ [rollup] Already fresh
✅ [build] No command to execute
1 file(s) are ASCII-safe.