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

The default 'alert' module no longer displays rendered HTML data for 'title' or 'message' parameters #3064

Closed
nfogal opened this issue Mar 12, 2023 · 7 comments

Comments

@nfogal
Copy link
Contributor

nfogal commented Mar 12, 2023

I found a bug in MagicMirror

Platform:

  • Raspberry Pi 4 8GB
  • Raspbian GNU/Linux 11 (bullseye)
  • Mozilla/5.0 Chrome/108.0.5359.62
  • Electron/22.0.0

Node Version: 18.15.0

MagicMirror² Version: 2.22.0

Description: The default 'alert' module and documentation (https://docs.magicmirror.builders/modules/alert.html) state that 'title' and 'message' parameters can contain either 'text' or 'html' data. However, when either 'title' or 'message' contain HTML, the HTML tags are displayed as text strings and they are not rendered. This issue was also brought to attention on the forum post at: https://forum.magicmirror.builders/topic/16336/alert-module-no-longer-accepts-html-for-notifications?_=1678593148680

Steps to Reproduce:

From any module, broadcast a notification / alert that contains HTML data in either the 'title' or 'message' parameters such as:

self.sendNotification("SHOW_ALERT", {title: '<u>Welcome</u>', message: '<b>Hello, Nick</b>'});

or

self.sendNotification('SHOW_ALERT', {type: 'notification', title: '<u>Welcome</u>', message: '<b>Hello, Nick</b>'});

Expected Results: The 'alert' module should display rendered HTML for both 'title' and / or 'message' parameters

Actual Results: Both 'text' and 'message' parameters are rendered as text strings (e.g. "Welcome" does not show an underlined title and "Hello, Nick" does not show the text as bold. Instead, each display the HTML tags to the interface as strings)

Configuration:

Additional Notes: This was more than likely caused by the updates to use nunjucks templates over straight HTML as stated in the forum post. I have a fix already in progress that I'll be pushing up shortly to the repo.

@khassel
Copy link
Collaborator

khassel commented Mar 12, 2023

Is this still an issue when you use the develop branch?

I'm asking because of #2828 which was fixed with #3018

@nfogal
Copy link
Contributor Author

nfogal commented Mar 13, 2023

Thanks @khassel for pointing out the develop branch, I didn't realize it existed.

It is still an issue with the 'title' parameter but the 'message' parameter has been fixed with #3018

The 'title' parameter can be fixed using the same approach of using the 'safe' filter

I'm also wondering if we want to use the 'safe' filter by default without giving the user the option to disable it (?) If, for whatever reason, the user wants to display for example "<iframe id="myWebsite"> is returning a 404" as an alert, the iframe tag would be rendered instead of a text string

@nfogal
Copy link
Contributor Author

nfogal commented Mar 13, 2023

My proposal would be to render 'title' and 'message' as HTML by default unless the notification payload contains 'titleType' / 'messageType' as 'text' such as:

this.sendNotification('SHOW_ALERT', {type: 'notification', title: '<iframe id="myWebsite"> is returning a 404', titleType: 'text', message: '<iframe id="myWebsite"> is returning a 404 and needs further investigation', messageType: 'text'});

@nfogal
Copy link
Contributor Author

nfogal commented Mar 13, 2023

alert.njk

{% if imageUrl or imageFA %}
  {% set imageHeight = imageHeight if imageHeight else "80px" %}
  {% if imageUrl %}
    <img src="{{ imageUrl }}" height="{{ imageHeight }}" style="margin-bottom: 10px;"/>
  {% else %}
    <span class="bright fas fa-{{ imageFA }}" style='margin-bottom: 10px; font-size: {{ imageHeight }};'/></span>
  {% endif %}
  <br/>
{% endif %}
{% if title %}
  <span class="thin dimmed medium">{{ title if titleType == 'text' else title | safe }}</span>
{% endif %}
{% if message %}
  {% if title %}
    <br/>
  {% endif %}
  <span class="light bright small">{{ message if messageType == 'text' else message | safe }}</span>
{% endif %}

@nfogal
Copy link
Contributor Author

nfogal commented Mar 13, 2023

notification.njk

{% if title %}
  <span class="thin dimmed medium">{{ title if titleType == 'text' else title | safe }}</span>
{% endif %}
{% if message %}
  {% if title %}
    <br/>
  {% endif %}
  <span class="light bright small">{{ message if messageType == 'text' else message | safe }}</span>
{% endif %}

@khassel
Copy link
Collaborator

khassel commented Mar 13, 2023

LGTM but please provide a PR (against develop) so we can see the diffs directly, thanks.

@nfogal
Copy link
Contributor Author

nfogal commented Mar 14, 2023

I made slight changes to render HTML by default as that is more preferred and avoids breaking the current behavior. PR is listed above

rejas pushed a commit that referenced this issue Mar 19, 2023
Fixes [#3064](#3064)

- Fixes default alert module nunjucks templates to render HTML by
default unless 'titleType' and 'messageType' are set to 'text' in the
payload data

e.g. 

Display Text:
`this.sendNotification('SHOW_ALERT', {type: "notification", title:
"<u>YoLink LeakSensor</u>", titleType: "text", message: "<b>" +
deviceName + "</b> reported an alarm that needs attention.",
messageType: "text"});`

Display HTML:
`this.sendNotification('SHOW_ALERT', {type: "notification", title:
"<u>YoLink LeakSensor</u>", message: "<b>" + deviceName + "</b> reported
an alarm that needs attention."});`
@rejas rejas added this to the 2.23 milestone Mar 19, 2023
@rejas rejas closed this as completed Apr 4, 2023
nfogal added a commit to nfogal/MagicMirror-Documentation that referenced this issue Apr 5, 2023
Update alert / notification table to include 'titleType' and 'messageType' based on MagicMirror Issue MagicMirrorOrg/MagicMirror#3064
rejas added a commit to MagicMirrorOrg/MagicMirror-Documentation that referenced this issue Jan 2, 2024
* Update alert.md

Update alert / notification table to include 'titleType' and 'messageType' based on MagicMirror Issue MagicMirrorOrg/MagicMirror#3064

* Update alert.md

Updated to move important from messageType to message

* Cleanup and add default values

---------

Co-authored-by: veeck <michael.veeck@nebenan.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants