Skip to content

Inline single-line event handlers in the web/app.js file#18527

Merged
timvandermeij merged 1 commit into
mozilla:masterfrom
Snuffleupagus:app-inline-short-listeners
Aug 2, 2024
Merged

Inline single-line event handlers in the web/app.js file#18527
timvandermeij merged 1 commit into
mozilla:masterfrom
Snuffleupagus:app-inline-short-listeners

Conversation

@Snuffleupagus

@Snuffleupagus Snuffleupagus commented Jul 31, 2024

Copy link
Copy Markdown
Collaborator

We have a fair number of (effectively) single-line event handlers in the web/app.js file, which leads to unnecessarily verbose code. These can, without affecting readability too much, be replaced either by:

  • Using bind for the simplest cases.
  • Using arrow-functions for the remaining ones.

Note that this is possible since we started removing event listeners with AbortSignal, which means that we no longer need to keep a reference to the event handler functions to be able to remove them.

Given that the old event handler functions use fairly long function names, and the way that they access PDFViewerApplication (given their scope), they impact the overall code-size unnecessarily.
Note: This patch reduces the size of the gulp mozcentral output by ~3.7 kilo-bytes, which isn't a lot but still cannot hurt.

@Snuffleupagus Snuffleupagus force-pushed the app-inline-short-listeners branch 2 times, most recently from 83b143a to 879d0bb Compare August 1, 2024 07:53
We have a fair number of (effectively) single-line event handlers in the  `web/app.js` file, which leads to unnecessarily verbose code. These can, without affecting readability too much, be replaced either by:
 - Using `bind` for the simplest cases.
 - Using arrow-functions for the remaining ones.

Note that this is possible since we started removing event listeners with `AbortSignal`, which means that we no longer need to keep a reference to the event handler functions to be able to remove them.

Given that the old event handler functions use fairly long function names, and the way that they access `PDFViewerApplication` (given their scope), they impact the overall code-size unnecessarily.
*Note:* This patch reduces the size of the `gulp mozcentral` output by `~3.7` kilo-bytes, which isn't a lot but still cannot hurt.
@Snuffleupagus Snuffleupagus force-pushed the app-inline-short-listeners branch from 879d0bb to 89f3a26 Compare August 1, 2024 20:03
@mozilla mozilla deleted a comment from moz-tools-bot Aug 1, 2024
@mozilla mozilla deleted a comment from moz-tools-bot Aug 1, 2024
@mozilla mozilla deleted a comment from moz-tools-bot Aug 1, 2024
@mozilla mozilla deleted a comment from moz-tools-bot Aug 1, 2024
@moz-tools-bot

Copy link
Copy Markdown
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_integrationtest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/377aa1b13f4ff09/output.txt

@moz-tools-bot

Copy link
Copy Markdown
Collaborator

From: Bot.io (Windows)


Received

Command cmd_integrationtest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.193.163.58:8877/c0ee1d01c7b2f70/output.txt

@moz-tools-bot

Copy link
Copy Markdown
Collaborator

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/377aa1b13f4ff09/output.txt

Total script time: 8.46 mins

  • Integration Tests: Passed

@moz-tools-bot

Copy link
Copy Markdown
Collaborator

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/c0ee1d01c7b2f70/output.txt

Total script time: 17.45 mins

  • Integration Tests: FAILED

@timvandermeij

Copy link
Copy Markdown
Collaborator

/botio-linux preview

@moz-tools-bot

Copy link
Copy Markdown
Collaborator

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/e4a5c1f0315c219/output.txt

@moz-tools-bot

Copy link
Copy Markdown
Collaborator

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/e4a5c1f0315c219/output.txt

Total script time: 1.04 mins

Published

@timvandermeij timvandermeij merged commit b80e552 into mozilla:master Aug 2, 2024
@timvandermeij

Copy link
Copy Markdown
Collaborator

Looks much better and easier to manage like this; nice work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants