Bug 36461 - Advanced editor should disable RequireJS timeout with waitSeconds: 0
Summary: Advanced editor should disable RequireJS timeout with waitSeconds: 0
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Phil Ringnalda
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-03-28 22:57 UTC by Phil Ringnalda
Modified: 2024-05-28 16:27 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.06,23.05.12


Attachments
Bug 36461: Advanced editor should disable RequireJS timeout with waitSeconds: 0 (1.78 KB, patch)
2024-03-28 23:01 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 36461: Advanced editor should disable RequireJS timeout with waitSeconds: 0 (1.84 KB, patch)
2024-03-29 12:14 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Phil Ringnalda 2024-03-28 22:57:45 UTC
We currently set waitSeconds: 30 for RequireJS in the advanced editor, which as bug 35799 says just causes loading of the editor to hang forever if loading any required part (which practially speaking means loading svc/cataloguing/framework) takes more than 30 seconds.

As jrburke says in https://github.com/requirejs/requirejs/issues/787 waitSeconds exists to show developers why things aren't working when they try to require something that 404s, and to allow error handling in old versions of IE. But since we aren't doing any error handling, we'd be better off setting it to 0, which disables the timeout, and just letting things load no matter how long they take.
Comment 1 Phil Ringnalda 2024-03-28 23:01:18 UTC
Created attachment 164099 [details] [review]
Bug 36461: Advanced editor should disable RequireJS timeout with waitSeconds: 0

We aren't actually doing any error handling with the RequireJS waitSeconds
config, just leaving the advanced editor hanging on loading if it times out,
so instead we should disable it.

Test plan:

1. Set the preference EnableAdvancedCatalogingEditor to Enable
2. Load the Cataloging module home page in Firefox and open Firefox's
   Web Developer Tools, and in the Network tab change from "No Throttling"
   to "Regular 2G"
3. In the Cataloging page, click Advanced editor, and watch it attempt to
   load for 30 seconds, followed by a setTimout error in the devtools console,
   and then wait as long as you can stand for anything but the "Loading,
   please wait..." to happen in the web page. Nothing ever will.
4. Apply the patch, restart, set EnableAdvancedCatalogingEditor back to
   Enable, turn throttling on, load the Advanced editor again, and note that
   even though it will take a minute or more to load through the throttling,
   the editor does actually load.
Comment 2 Nick Clemens (kidclamp) 2024-03-29 12:14:13 UTC
Created attachment 164125 [details] [review]
Bug 36461: Advanced editor should disable RequireJS timeout with waitSeconds: 0

We aren't actually doing any error handling with the RequireJS waitSeconds
config, just leaving the advanced editor hanging on loading if it times out,
so instead we should disable it.

Test plan:

1. Set the preference EnableAdvancedCatalogingEditor to Enable
2. Load the Cataloging module home page in Firefox and open Firefox's
   Web Developer Tools, and in the Network tab change from "No Throttling"
   to "Regular 2G"
3. In the Cataloging page, click Advanced editor, and watch it attempt to
   load for 30 seconds, followed by a setTimout error in the devtools console,
   and then wait as long as you can stand for anything but the "Loading,
   please wait..." to happen in the web page. Nothing ever will.
4. Apply the patch, restart, set EnableAdvancedCatalogingEditor back to
   Enable, turn throttling on, load the Advanced editor again, and note that
   even though it will take a minute or more to load through the throttling,
   the editor does actually load.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 3 Nick Clemens (kidclamp) 2024-03-29 12:58:31 UTC
Trivial update - we aren't handling the error, so waiting will appear the same, but it may succeed now
Comment 4 Katrin Fischer 2024-04-04 13:25:54 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 5 Fridolin Somers 2024-05-23 09:11:49 UTC
Pushed to 23.11.x for 23.11.06
Comment 6 Lucas Gass 2024-05-28 16:27:06 UTC
Backported to 23.05.x for upcoming 23.05.12