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.

Sign up for GitHub

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

Jump to bottom

Fix bug that caused & to display and improve one-line rich-text fields #4267

Merged
merged 12 commits into from
Jun 19, 2024
Merged

Fix bug that caused & to display and improve one-line rich-text fields #4267

merged 12 commits into from
Jun 19, 2024

Conversation

joemull
Copy link
Member

@joemull joemull commented Jun 10, 2024 •
edited
Loading

Closes #4258.

This fixes a bug with ampersands reported in the issue above.

It also improves usability and handling of a number of related things:

  • Adds a Janeway custom bleach form field (previously we just had a custom bleach model field)
  • Changes the bleaching of setting to depend on setting type, meaning that charfields, text fields, numbers etc. are left alone--not even escaping of ampersands is applied
  • Adds a new setting type for one-line HTML fields, called "mini HTML"
  • Users can now edit one-line rich-text fields like article title with a small TinyMCE widget
  • Documents the four main text-related field types in Janeway fields and settings
  • Standardizes the setting types for settings that should use text, char, or mini-html to the appropriate type

@joemull joemull changed the base branch from master to release_1_5_x June 10, 2024 14:06
joemull added 7 commits June 10, 2024 17:23
@joemull
Allow filtering setting values by type in admin #4258
1378f20
@joemull
Define JanewayBleachFormField and MiniHTMLFormField #4258
00048d7
@joemull
Establish mini-html setting type and do cleanup #4258
08cf695
@joemull
TinyMCE should only escape required entities #4258
49c8570 
By default, TinyMCE performs escaping on some characters
entered via the rich-text editing interface
(not the code view).

We want minimal escaping, so entity_encoding = 'raw'.

The TinyMCE docs on raw entity encoding:

All characters will be stored in non-entity form except
these XML default entities: &, <, >, ', and ".
@joemull
Bleach setting value conditionally by type #4258
5cacb3d
@joemull
Document our text field types #4258
c0ffec3
@joemull
Prefer char type for metadata fields #4258
aa263db
@joemull joemull force-pushed the 4258-bleach-and-html-entities branch from 7b8aff3 to aa263db Compare June 11, 2024 12:15
@joemull joemull changed the title Handle Unicode and limited phrasing markup in bleached character fields Fix bug that caused &amp; to display and improve one-line rich-text fields Jun 11, 2024
@joemull joemull marked this pull request as ready for review June 11, 2024 12:20
@joemull joemull requested a review from mauromsl June 11, 2024 12:20
@joemull joemull changed the title Fix bug that caused &amp; to display and improve one-line rich-text fields Fix bug that caused &amp; to display and improve one-line rich-text fields Jun 11, 2024
@joemull joemull assigned ajrbyers and unassigned mauromsl Jun 11, 2024
@joemull joemull requested review from ajrbyers and removed request for mauromsl June 11, 2024 12:26
@joemull joemull requested a review from mauromsl June 11, 2024 12:27
ajrbyers
ajrbyers requested changes Jun 11, 2024
View reviewed changes
Copy link
Member

@ajrbyers ajrbyers left a comment

Choose a reason for hiding this comment

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

Just one comment, looks good.

src/core/forms/forms.py Outdated Show resolved Hide resolved
@ajrbyers ajrbyers assigned joemull and unassigned ajrbyers and mauromsl Jun 11, 2024
@joemull joemull assigned ajrbyers and unassigned joemull Jun 11, 2024
@joemull joemull requested a review from ajrbyers June 11, 2024 13:28
mauromsl
mauromsl requested changes Jun 11, 2024
View reviewed changes
Copy link
Member

@mauromsl mauromsl left a comment

Choose a reason for hiding this comment

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

Very happy with this implementation. Well done @joemull.

As mentioned inline, it'd be good to fix any potentially corrupt journal title due to entity encoding (it is probably safe to only replace "&" but wouldn't harm to also fix encoded "<" and ">")

Couple minor comments regarding some class initialisers

src/core/migrations/0089_update_setting_types.py Outdated Show resolved Hide resolved
src/core/migrations/0089_update_setting_types.py Outdated Show resolved Hide resolved
src/core/model_utils.py Show resolved Hide resolved
src/core/model_utils.py Outdated Show resolved Hide resolved
src/core/model_utils.py Outdated Show resolved Hide resolved
@mauromsl mauromsl assigned joemull and unassigned mauromsl and ajrbyers Jun 11, 2024
@joemull joemull requested a review from mauromsl June 12, 2024 11:56
@joemull joemull assigned mauromsl and ajrbyers and unassigned joemull Jun 12, 2024
@joemull joemull mentioned this pull request Jun 12, 2024
Merged
@joemull joemull added this to the v1.5.6 milestone Jun 17, 2024
@ajrbyers ajrbyers removed their assignment Jun 18, 2024
mauromsl
mauromsl approved these changes Jun 19, 2024
View reviewed changes
src/core/model_utils.py Show resolved Hide resolved
@mauromsl mauromsl merged commit 6918b72 into release_1_5_x Jun 19, 2024
1 check failed
@mauromsl mauromsl deleted the 4258-bleach-and-html-entities branch June 19, 2024 16:09
@mauromsl mauromsl mentioned this pull request Jun 19, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ajrbyers ajrbyers ajrbyers approved these changes

@mauromsl mauromsl mauromsl approved these changes

Assignees

@mauromsl mauromsl

Labels
None yet
Projects
None yet
Milestone
v1.5.6
Development

Successfully merging this pull request may close these issues.

HTML entities in bleached settings are not all rendered properly
3 participants
@joemull @ajrbyers @mauromsl