Skip to content
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

Fix HIX value difference #2302

Merged
merged 1 commit into from
May 30, 2023
Merged

Fix HIX value difference #2302

merged 1 commit into from
May 30, 2023

Conversation

MizukiTemma
Copy link
Member

@MizukiTemma MizukiTemma commented May 24, 2023

Short description

This PR tries to keep the HIX value identical between pre save signal and tinymce.

Proposed changes

  • replace \n with \r\n before checking HIX value.
  • replace entity reference with numeric reference (example: ä -> ä) before checking HIX value.

Side effects

  • Sadly I couldn't find a way to convert entity reference to numeric reference directly and needed some converting steps.
  • It seems, so far observed in the test data, reference art of special letters does not cause difference in HIX value, but the style of new line does.
  • If "\n" appears in the text by chance, for example when an user writes "Please use \n as line break" in the editor, simple replacement might replace it too, even though it is not intended as a line break. I testet it and "\n" tipped in the editor was not replaced, but I cannot garant it...

Resolved issues

Fixes: #2224


Pull Request Review Guidelines

@MizukiTemma MizukiTemma requested a review from a team as a code owner May 24, 2023 15:32
@codeclimate
Copy link

codeclimate bot commented May 24, 2023

Code Climate has analyzed commit 64eab46 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 28.5% (50% is the threshold).

This pull request will bring the total coverage in the repository to 76.2%.

View more on Code Climate.

@timobrembeck timobrembeck added the ‼️ prio: high Needs to be resolved ASAP. label May 24, 2023
Copy link
Member

@timobrembeck timobrembeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks a lot! 🙏

integreat_cms/cms/views/utils/hix.py Outdated Show resolved Hide resolved
integreat_cms/cms/views/utils/hix.py Outdated Show resolved Hide resolved
Co-authored-by: Timo Ludwig <timo.ludwig@tuerantuer.org>
@MizukiTemma MizukiTemma force-pushed the bug/fix_hix_value_difference branch from d758d20 to 64eab46 Compare May 25, 2023 18:42
@JoeyStk JoeyStk self-requested a review May 29, 2023 08:13
Copy link
Contributor

@JoeyStk JoeyStk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this important fix. I wasn't able to break it 👍

@MizukiTemma MizukiTemma merged commit deca08b into develop May 30, 2023
5 checks passed
@MizukiTemma MizukiTemma deleted the bug/fix_hix_value_difference branch May 30, 2023 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
‼️ prio: high Needs to be resolved ASAP.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HIX value differs when calculated via tinymce vs pre save signal
3 participants