[MGNLUI-5337] Notifications and alerts should support HTML content Created: 21/Aug/19  Updated: 24/Oct/19  Resolved: 24/Oct/19

Status: Closed
Project: Magnolia UI
Component/s: admincentral
Affects Version/s: 6.1
Fix Version/s: 6.1.3, 6.2

Type: Bug Priority: Neutral
Reporter: Federico Grilli Assignee: Adam Siska
Resolution: Not an issue Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: PNG File html-code-shows.png    
Issue Links:
Relates
relates to MGNLUI-4608 Inline HTML leaks into language choos... Closed
duplicate
is duplicated by MAGNOLIA-7427 Line breaks are displayed as <br> in ... Closed
is duplicated by MGNLUI-5331 DAM Upload Success/Error popup Closed
relation
is related to MGNLUI-4760 Resurface: Inline HTML leaks into mes... Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:
Epic Link: Support
Sprint: UI Framework 6
Story Points: 3

 Description   

Notifications and alerts often contain HTML formatted text. Currently this seems not to be always supported by Magnolia (IIRC, Vaadin now disallows html content by default for security reasons). See attachment. More examples can be found in the two tickets linked as duplicate.

After proper text sanitisation on the server side, it should be safe to allow HTML content. Sanitisation is important here because the text displayed may partly come from the client and contain malicious executable code.

 

See info.magnolia.ui.AlertBuilder and info.magnolia.admincentral.ResurfaceShell#openNotification (for legacy notifs)



 Comments   
Comment by Mikaël Geljić [ 10/Sep/19 ]

I suggest to keep notifications (as opposed to alerts) as simple as possible, and simply avoid HTML in our translations.
Here for this case I would also remove the file name, it does not add any value.

Comment by Adam Siska [ 13/Sep/19 ]

https://jira.magnolia-cms.com/browse/MGNLUI-4608?focusedCommentId=186158&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-186158

The thing is that this is from compatibility framework, and tags are there for some time, they are also in some translations: https://git.magnolia-cms.com/plugins/servlet/search?q=project%3ALANG%20repo%3Alang%20field.upload.note.success

Notification in new UI could be set on-place with:

public Notification(String caption, String description, Type type, boolean htmlContentAllowed)

The question is should we enable HTML for all (compatibility fw) ResurfaceShell:openNotification() ?

BTW FYI: HTML is enabled already in body of alerts (not header nor buttons labels).

Comment by Federico Grilli [ 13/Sep/19 ]

My two cents is that nothing prevents us from allowing HTML content in alerts and notifs (old and new), provided we first sanitise the text which may come from user input.
Of course, I agree with Mika and we should aim at keeping notifs as simple as possible but there are so many already in place that it would require some time to go through and clean them up all. Also, sometimes a little formatting make sense, as it helps readability. 

Comment by Adam Siska [ 13/Sep/19 ]

> "there are so many already in place"
actually there are not many of them (like 20 in ui, but with already existing translations..)

> "sometimes a little formatting make sense, as it helps readability"
I used org.jsoup.Jsoup.Cleaner with basic whitelist as it is used on translations

Comment by Adam Siska [ 24/Sep/19 ]

This was reverted, so UiContext::showNotification don't support HTML. These are all messages in UI project containing HTML, only field.upload.* go into notifications in old Assets app in 6.1, iirc (and their translations of course).

----------------------------
File app-security-messages_en.properties contains:
security.delete.group.isAssignedError=It is already assigned to the following users/groups:<br />
security.delete.role.isAssignedError=It is already assigned to the following users/groups:<br />
----------------------------
 
----------------------------
File about-app-messages_en.properties contains:
about.app.mapping.virtual.uri.mappings.deprecated.title = Open the <a href="#placeholder">Definitions app</a> to browse Virtual URI Mappings
about.app.main.privacy.label=Send anonymous usage statistics to Magnolia to make the product better. See <a href="http://mgnl.io/privacy" target="_blank">Privacy policy</a>
----------------------------
 
----------------------------
File ui-admincentral-messages_en.properties contains:
confirmation.delete.message = The {0} <br> {1} <br> will be deleted immediately.<br>Are you sure?
confirmation.mark.delete.message = The {0} <br> {1} <br> and its sub nodes will be marked for deletion.<br> Are you sure?
validation.message.errors          = Please correct the <strong># errors</strong> in this form
----------------------------
 
----------------------------
File uploadField-messages_en.properties contains:
field.upload.drag.hint = or <em>drag an image into this area</em> to upload it
field.upload.basic.drop.hint = or <em>drag a file into this area</em> to upload it
field.upload.basic.m6.uploading.file=Uploading file <b>{0}</b>
field.upload.drop.hint = or <em>drag a file into this area</em> to upload it
field.upload.note.success = Your file has been uploaded successfully<br>{0}
field.upload.note.warning = The upload of your file was aborted due to<br>{0}
field.upload.note.error = An error occurred while uploading your file<br>{0}
field.upload.drop.hint.media = or <em>drag {0} into this area</em> to upload it
field.upload.note.success.media = Your {0} has been uploaded successfully<br>{1}
field.upload.note.warning.media = The upload of your {0} was aborted due to<br>{1}
field.upload.note.error.media = An error occurred while uploading your {0}<br>{1}
----------------------------
 
----------------------------
File module-ui-contentapp-messages_en.properties contains:
ui-contentapp.actions.restoreVersion.confirmation.body=You are about to restore the <em>oldest</em> version. Do you want to restore it without creating a version of the current content?
----------------------------
 
----------------------------
File module-ui-framework-messages_en.properties contains:
confirmation.delete.modified.message=<b>Warning</b><br/>The following sub-nodes have been modified but not published. If they have been moved from a different location, they won't be properly removed from public instances.<br/><br/>Modified sub-nodes:
----------------------------
 
----------------------------
File module-ui-dialog-messages_en.properties contains:
languageSelector.description=<b>Content must be valid to switch language.</b><br/>If you are switching to an incomplete language, invalid fields will have to be updated before saving. Alternatively, consider saving pending changes first.

Generated at Mon Feb 12 09:25:31 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.