Opened 9 years ago

Closed 8 years ago

Last modified 8 years ago

#25165 closed Cleanup/optimization (fixed)

Move JavaScript calls out of HTML to fix JavaScript "no-script-eval" warnings

Reported by: Tim Graham Owned by: Tim Graham <timograham@…>
Component: contrib.admin Version: dev
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


Run the JavaScript tests and you'll see some warnings about "Script URL is a form of eval". These should be replaced with click handlers (href="#" and then use id or class attributes to select the element in the JavaScript and register a click handler there, with "return false" to stop event propagation).

Change History (14)

comment:1 by Rigel Di Scala, 9 years ago

Owner: changed from nobody to Rigel Di Scala
Status: newassigned

Will take a look.

comment:2 by Tim Graham, 9 years ago

Has patch: set
Owner: Rigel Di Scala removed
Status: assignednew

comment:3 by Tim Graham <timograham@…>, 8 years ago

Owner: set to Tim Graham <timograham@…>
Resolution: fixed
Status: newclosed

In d638cdc:

Fixed #25165 -- Removed inline JavaScript from the admin.

This allows setting a Content-Security-Policy HTTP header
(refs #15727).

Special thanks to blighj, the original author of this patch.

comment:4 by Tim Graham, 8 years ago

Has patch: unset
Resolution: fixed
Severity: NormalRelease blocker
Status: closednew

Hi Thomas, I noticed a regression due to this change. Go to the admin change form of an object with a foreign key and use the "Add another" button on the foreign key field. After creating a new object using the popup, the popup doesn't close and the JavaScript console displays TypeError: text.replace is not a function. Are you able to investigate?

comment:5 by Tim Graham, 8 years ago

The selenium test added in f18b08748abaecb171fdcbcdfdcd7e4d95d931d1 can likely be extended for this fix.

comment:6 by Tim Graham, 8 years ago

Another regression: when using the "Add another" button on a foreign key to a UUIDField in the admin, the JSON serialization will fail with TypeError: UUID('f9cd8eb4-7c00-407b-bc35-17a8a8d0b9f0') is not JSON serializable.

comment:7 by Tim Graham, 8 years ago

PR for the second regression (waiting to corporate the tests from a patch for the stable/1.9.x branch as noted there).

comment:8 by Tim Graham, 8 years ago

Has patch: set

PR for the issue in comment 4.

comment:9 by Tim Graham <timograham@…>, 8 years ago

In 822a03b3:

Refs #25165 -- Fixed failure of admin's "Add another" popup to close.

Thanks Thomas Grainger for the fix.

comment:10 by Tim Graham <timograham@…>, 8 years ago

In ade54ff:

Refs #25165 -- Fixed JSON serialization for add/edit popup in the admin.

Forwardport of test in o839d71d8562abe0b245024e55ca1d02a45e58fd from stable/1.9.x
(refs #25997).

comment:11 by Tim Graham, 8 years ago

Resolution: fixed
Status: newclosed

comment:12 by Tim Graham <timograham@…>, 8 years ago

In cbaa3ee:

Refs #25165 -- Removed unnecessary HTML unescaping in admin add/edit popups.

Because we now load data into the page via JSON, we don't need to
unescape it anymore.

comment:13 by Tim Graham, 8 years ago

And to fix serialization for the delete popup: PR

comment:14 by Tim Graham <timograham@…>, 8 years ago

In 3541ca15:

Refs #25165 -- Fixed JSON serialization for delete popup in the admin.

Note: See TracTickets for help on using tickets.
Back to Top