Opened 4 years ago

Last modified 4 years ago

#14545 confirmed New Feature

editor.showNotification doesn't handle line breaks

Reported by: Alfonso Martínez de Lizarrondo Owned by:
Priority: Normal Milestone:
Component: General Version: 4.5.0
Keywords: Cc:

Description

Steps to reproduce

  1. Load ckeditor.com/demo
  2. Open the console and execute
CKEDITOR.instances.editor1.showNotification("Hi\r\nWhat's up?")
  1. Compare with an editor without the notification plugin that will end up calling window.alert:
alert("Hi\r\nWhat's up?")

Expected result

The notification plugin should convert linebreaks into <br> so that any call to editor.showNotification is displayed in a similar way.

Actual result

alert shows the message in two lines, the html notification uses only one.

Other details (browser, OS, CKEditor version, installed plugins)

Introduced with the notifications plugin, I'm guessing 4.5 at least

Change History (5)

comment:1 Changed 4 years ago by Jakub Ś

Status: newconfirmed
Type: BugNew Feature

Multiline notifications were never our plan. Also when you enter something like CKEDITOR.instances.editor1.showNotification("Hi <br> What's up?") the HTML is properly interpreted.

Because of two above I see this more a feature request than a actual bug and will confirm this ticket as such.

comment:2 Changed 4 years ago by Alfonso Martínez de Lizarrondo

Using the <br> works if the Notification plugin is loaded, otherwise the executed code is this one from editor.js:

		showNotification: function( message ) {
			alert( message ); // jshint ignore:line
		}

then the user will see a <br> instead of a newline.

So in the current situation a call to editor.showNotification must detect if the Notification plugin is loaded or not to know if it must replace the \r\n with <br> or not, otherwise it will be shown incorrectly in the other situation.

This is the only conversion that I would do as the goal is just to switch from alert to showNotification, and so I think that any HTML in the message should be escaped, but that's a matter for another bug and another debate.

comment:3 Changed 4 years ago by Jakub Ś

Looking at the comment I assume that you are creating a custom plugin which may/may not use notifications.
At the moment multiline is not supported. The fact that br's work is that because you pass text to notification and it gets interpreted as HTML in browser. I agree that it would be nice if notification would work on more general level i.e. convert br's to /r/n in general showNotification function and vice versa in notifications plugin showNotification function.
This would definitely helped for cases like yours because I imagine that there might be more people in the future which will create plugins which relay on notification plugin or its default function.

comment:4 Changed 4 years ago by Marek Lewandowski

Yes, current situation where we expect HTML message, and put it into alert if no extra plugin is available, is not consistent and should be improved.

Accepting HTML is fine, because allow rich content in the notification. As such we could convert br to line breaks, and simply strip other HTML tags in default implementation. It's important to remove any extra line breaks before converting <br>'s, as one might put formatted HTML with line breakings.

comment:5 Changed 4 years ago by Jakub Ś

Once this is done, it will probably be worth mentioning in docs that by default we expect HTML in notification message but if there is no notification plugin HTML will be removed and br's will be converted to /n/r.

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy