Ensure that TextLayerBuilder works correctly without the abortSignal parameter (PR 20928 follow-up)

[Snuffleupagus]

After the changes in PR #20928 the code no longer works correctly unless the abortSignal parameter is provided, which completely breaks text-selection in e.g. the standalone viewer-components with errors such as:

 #renderTextLayer: TypeError: EventTarget.addEventListener: 'signal' member of AddEventListenerOptions is not an object.
    #bindMouse http://localhost:8888/web/text_layer_builder.js:173
    render http://localhost:8888/web/text_layer_builder.js:128
    #renderTextLayer http://localhost:8888/web/pdf_page_view.js:532
    resultPromise http://localhost:8888/web/pdf_page_view.js:1184
    promise callback*draw http://localhost:8888/web/pdf_page_view.js:1174
    renderView http://localhost:8888/web/pdf_rendering_queue.js:219
    forceRendering http://localhost:8888/web/pdf_viewer.js:2081
    promise callback*forceRendering http://localhost:8888/web/pdf_viewer.js:2080
    renderHighestPriority http://localhost:8888/web/pdf_rendering_queue.js:84
    update http://localhost:8888/web/pdf_viewer.js:1895
    onScaleChanging http://localhost:8888/web/app.js:2755
    dispatch http://localhost:8888/web/event_utils.js:115
    #setScaleUpdatePages http://localhost:8888/web/pdf_viewer.js:1555
    #setScale http://localhost:8888/web/pdf_viewer.js:1640
    set currentScaleValue http://localhost:8888/web/pdf_viewer.js:592
    setInitialView http://localhost:8888/web/app.js:1969
    load http://localhost:8888/web/app.js:1570
    promise callback*load/< http://localhost:8888/web/app.js:1518
    promise callback*load http://localhost:8888/web/app.js:1507
    open http://localhost:8888/web/app.js:1255
    promise callback*open http://localhost:8888/web/app.js:1253
    run http://localhost:8888/web/app.js:895
    webViewerLoad http://localhost:8888/web/viewer.js:366
    <anonymous> http://localhost:8888/web/viewer.js:377
pdf_page_view.js:547:15

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.66%. Comparing base (ab228da) to head (ba796a3).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #20934      +/-   ##
==========================================
- Coverage   62.72%   62.66%   -0.06%     
==========================================
  Files         173      173              
  Lines      121500   121502       +2     
==========================================
- Hits        76205    76137      -68     
- Misses      45295    45365      +70     

Flag	Coverage Δ
fonttest	?
unittestcli	62.66% <ø> (-0.04%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[timvandermeij]

/botio-linux preview

[moz-tools-bot]

From: Bot.io (Linux m4)

---

Received

Command cmd_preview from @timvandermeij received. Current queue size: 0

Live output at: http://54.241.84.105:8877/88376862c3db7e2/output.txt

[moz-tools-bot]

From: Bot.io (Linux m4)

---

Success

Full output at http://54.241.84.105:8877/88376862c3db7e2/output.txt

Total script time: 1.17 mins

Published

• Viewer: http://54.241.84.105:8877/88376862c3db7e2/web/viewer.html
• Viewer (legacy): http://54.241.84.105:8877/88376862c3db7e2/legacy/web/viewer.html

[timvandermeij]

Thank you for spotting this!

[calixteman]

Is there a way to have some tests in order to avoid such regressions ? For example we could write (if it doesn't exist) a babel plugin in order to check at least that the number of args is correct.

[Snuffleupagus]

Is there a way to have some tests in order to avoid such regressions ?

The quickest solution would probably be to make it easier to test something along the lines of the simpleviewer example during development.

For example we could write (if it doesn't exist) a babel plugin in order to check at least that the number of args is correct.

I don't know what sort of Babel plugin you have in mind, but I'm not sure it's easy to detect this situation.

Note that the problem here was that if abortSignal isn't explicitly set, we basically ended up with the following code:

div.addEventListener(
  "evtname",
  evt => { ... },
  { signal: null } // This throws.
);

[calixteman]

And what about adding an integration test with simpleviewer ?

[Snuffleupagus]

And what about adding an integration test with simpleviewer ?

Sure, that might work.

However, using that one directly probably isn't a good idea since running pdfjs dist-install as part of testing seem way too heavy. Perhaps adding a "simple" viewer, similar to how the new debugger was implemented, and building that during testing could work.