-
Notifications
You must be signed in to change notification settings - Fork 590
chore: Bundle js files, remove jQuery, and static libs #1556
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry @fsbraun, your pull request is larger than the review limit of 150000 diff characters
| attrs["class"] = "vForeignKeyRawIdAdminField" | ||
| super_attrs = attrs.copy() | ||
| hidden_input = super(ForeignKeyRawIdWidget, self).render(name, value, super_attrs) # grandparent super | ||
| hidden_input = super(ForeignKeyRawIdWidget, self).render( |
Check failure
Code scanning / CodeQL
First argument to super() is not enclosing class Error
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
To fix the problem, update the call to super() on line 51 in the render method so that the first argument is the enclosing class: AdminFolderWidget. Thus, change super(ForeignKeyRawIdWidget, self).render(...) to super(AdminFolderWidget, self).render(...). This ensures that if the class is used in a multiple inheritance scenario, or extended, Python's MRO will correctly traverse parent classes, calling render() from the correct superclass.
Only line 51 inside filer/fields/folder.py needs modification. No new imports, methods, or definitions are required.
-
Copy modified line R51
| @@ -48,7 +48,7 @@ | ||
| # The JavaScript looks for this hook. | ||
| attrs["class"] = "vForeignKeyRawIdAdminField" | ||
| super_attrs = attrs.copy() | ||
| hidden_input = super(ForeignKeyRawIdWidget, self).render( | ||
| hidden_input = super(AdminFolderWidget, self).render( | ||
| name, value, super_attrs | ||
| ) # grandparent super | ||
|
|
| className = 'class="hidden"'; | ||
| }); | ||
| if (dropdown) { | ||
| dropdown.insertAdjacentHTML('beforeend', html); |
Check warning
Code scanning / CodeQL
DOM text reinterpreted as HTML Medium
DOM text
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
How to fix the problem in general:
Whenever DOM text or user data is injected into an HTML string (especially via innerHTML, insertAdjacentHTML, etc.), it must be properly escaped to prevent HTML interpretation and hence XSS. You must ensure all generated HTML is safe, which means escaping or encoding any dynamic content to prevent meta-characters being interpreted as markup.
Best way to fix:
Before interpolating option.textContent into the HTML, escape it so that any special HTML characters (&, <, >, ", ') are replaced with their HTML entity equivalents. This can be implemented with a simple utility function, escapeHTML. The escaping should be applied directly before interpolation so that malicious content cannot break out of the anchor's structure.
In this context, there is no need for external libraries—modern JS makes escaping straightforward.
Where to change:
- Introduce an
escapeHTMLfunction in this file. - Rreplace
${option.textContent}with${escapeHTML(option.textContent)}in the line building the HTML for the dropdown. - No further code changes are needed; the remainder of the code can stay the same.
-
Copy modified lines R16-R25 -
Copy modified line R214
| @@ -13,6 +13,16 @@ | ||
| Cl.Toggler = Toggler; | ||
|
|
||
|
|
||
| // Escape HTML utility to safely inject text into HTML | ||
| function escapeHTML(str) { | ||
| return str | ||
| .replace(/&/g, "&") | ||
| .replace(/</g, "<") | ||
| .replace(/>/g, ">") | ||
| .replace(/"/g, """) | ||
| .replace(/'/g, "'"); | ||
| } | ||
|
|
||
| document.addEventListener('DOMContentLoaded', () => { | ||
| let showErrorTimeout; | ||
|
|
||
| @@ -201,7 +211,7 @@ | ||
| if (option.value === valueDelete || option.value === valueCopy || option.value === valueMove) { | ||
| className = 'class="hidden"'; | ||
| } | ||
| html += `<li><a href="#"${className}>${option.textContent}</a></li>`; | ||
| html += `<li><a href="#"${className}>${escapeHTML(option.textContent)}</a></li>`; | ||
| } | ||
| }); | ||
| if (dropdown) { |
| var checkMinWidth = function (element) { | ||
| element.toggleClass(mobileClass, element.width() < minWidth); | ||
| document.addEventListener('DOMContentLoaded', () => { | ||
| const dropzoneTemplateSelector = '.js-filer-dropzone-template'; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
To fix the problem, remove the declaration of dropzoneTemplateSelector from line 12 in filer/static/filer/js/addons/dropzone.init.js.
- Only delete the line declaring the unused variable.
- No additional imports, methods, or definitions are required.
- This change does not affect functionality as the variable is unused.
| @@ -9,7 +9,6 @@ | ||
| } | ||
|
|
||
| document.addEventListener('DOMContentLoaded', () => { | ||
| const dropzoneTemplateSelector = '.js-filer-dropzone-template'; | ||
| const previewImageSelector = '.js-img-preview'; | ||
| const dropzoneSelector = '.js-filer-dropzone'; | ||
| const dropzones = document.querySelectorAll(dropzoneSelector); |
| if (clearButton) { | ||
| clearButton.addEventListener('click', () => { | ||
| dropzone.classList.remove(objectAttachedClass); | ||
| const changeEvent = new Event('change', { bubbles: true }); |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
To fix the problem, remove the unused local variable changeEvent defined on line 102, since it is never used in the code and only created for a commented-out statement. Locate the definition const changeEvent = new Event('change', { bubbles: true }); inside the click event handler for clearButton, and delete this line.
No additional imports, methods, or definitions are necessary. Ensure that only the line creating changeEvent is removed, without disturbing the surrounding logic.
| @@ -99,7 +99,6 @@ | ||
| if (clearButton) { | ||
| clearButton.addEventListener('click', () => { | ||
| dropzone.classList.remove(objectAttachedClass); | ||
| const changeEvent = new Event('change', { bubbles: true }); | ||
| // inputId?.dispatchEvent(changeEvent); | ||
| }); | ||
| } |
Description
Convert JavaScript in django-filer from jQuery to ES modules and vanilla JS, bundle scripts with webpack, remove static legacy libraries, and modernize the build and lint workflows.
New Features:
Enhancements:
Build:
CI:
Chores:
Related resources
Checklist
masterSlack to find a “pr review buddy” who is going to review my pull request.