Skip to content

Comments

Remove attribute only when necessary#1085

Merged
cure53 merged 1 commit intocure53:mainfrom
michalnieruchalski-tiugo:remove_attribute_when_necessary
Apr 11, 2025
Merged

Remove attribute only when necessary#1085
cure53 merged 1 commit intocure53:mainfrom
michalnieruchalski-tiugo:remove_attribute_when_necessary

Conversation

@michalnieruchalski-tiugo
Copy link
Contributor

Summary

The change is to make _sanitizeAttributes() only remove an attribute if needed. The way that attributes are sanitized is not changed.

Background & Context

This provides a performance benefit as attributes are not constantly removed and added back. It also prevents attribute re-ordering, making comparing the pre-sanitized and post-sanitzed content easier and simplifying writing tests. The engineers at Tiny have had a patch applied to dompurify with these changes in production for many years and would like to contribute the changes upstream.

Tests

As attributes are not removed and re-added, the original attribute order should be maintained after sanitization. As a result, the order of attributes in several tests has been updated. Nothing else about the test cases has been altered.

Tasks

  • Review my changes :)

Dependencies

N/A

@cure53
Copy link
Owner

cure53 commented Apr 11, 2025

Oha, that looks very interesting, thanks a lot! We'll give it a spin.

If all looks fine, we would release this as 3.3.0 as this might be a breaking change for some users.

@cure53 cure53 merged commit 1b09f31 into cure53:main Apr 11, 2025
8 checks passed
pparidans added a commit to odoo-dev/odoo that referenced this pull request Sep 29, 2025
Note: due to the PR [1], the attributes' order is more predicatable,
hence the tests adaptations.

[1]: cure53/DOMPurify#1085

task-5110128
robodoo pushed a commit to odoo/odoo that referenced this pull request Sep 30, 2025
Note: due to the PR [1], the attributes' order is more predicatable,
hence the tests adaptations.

[1]: cure53/DOMPurify#1085

task-5110128

closes #228417

Related: odoo/enterprise#95776
Signed-off-by: Aaron Bohy (aab) <[email protected]>
robodoo pushed a commit to odoo/odoo that referenced this pull request Sep 30, 2025
Note: due to the PR [1], the attributes' order is more predicatable,
hence the tests adaptations.

[1]: cure53/DOMPurify#1085

task-5110128

closes #228417

Related: odoo/enterprise#95776
Signed-off-by: Aaron Bohy (aab) <[email protected]>
kmagusiak pushed a commit to odoo-dev/odoo that referenced this pull request Oct 7, 2025
Note: due to the PR [1], the attributes' order is more predicatable,
hence the tests adaptations.

[1]: cure53/DOMPurify#1085

task-5110128

closes odoo#228417

Related: odoo/enterprise#95776
Signed-off-by: Aaron Bohy (aab) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants