DistributedProofreaders / dproofreaders

Distributed Proofreaders is a web application intended to ease the process of converting public domain books into e-texts.
https://www.pgdp.net
GNU General Public License v2.0
46 stars 28 forks source link

move text valdator script to avoid js errors #1229

Closed 70ray closed 2 weeks ago

70ray commented 2 weeks ago

Otherwise, in handle_bad_page, a type error occurs in text_validator.js when it tries to add an event listener to the 'cc-quit' button.

Sandbox at: https://www.pgdp.org/~rp31/c.branch/bad_page_js

70ray commented 2 weeks ago

Why does moving this from the header into the body fix the problem? If the issue is that the JS is trying to bind a listener to an ID that doesn't exist in this code path for the page, perhaps it would be better if the JS checked that the ID exists before it tries to bind the listener.

The validator is drawn by render_validator() inside the function show_text_update_form() so if the script text_validator.js is also only loaded if this php function is called then the ids it is looking for will exist.

This departs from the usual way of loading js files so I'm happy to do it the other way if you prefer.

chrismiceli commented 2 weeks ago

Why does moving this from the header into the body fix the problem? If the issue is that the JS is trying to bind a listener to an ID that doesn't exist in this code path for the page, perhaps it would be better if the JS checked that the ID exists before it tries to bind the listener.

The validator is drawn by render_validator() inside the function show_text_update_form() so if the script text_validator.js is also only loaded if this php function is called then the ids it is looking for will exist.

This departs from the usual way of loading js files so I'm happy to do it the other way if you prefer.

I think we'd be more consistent with our other pages (I think at least) if we just put some js conditional on the DOM structure the file expects in the cases it is useful. The approach you did here though does reduce page loads for cases where the js isn't needed.

70ray commented 2 weeks ago

Use alt_bad_page instead.