860951 - \n in XUL alert text doesn't create a newline

Status: VERIFIED FIXED

(Toolkit :: General, defect)

Description

[Reuben Morais [:reuben]]

Cc["@mozilla.org/alerts-service;1"].getService(Ci.nsIAlertsService).showAlertNotification(null, "Title", "Foo\nBar");

Actual results:
Foo Bar

Expected results:
Foo
Bar

Comment 1

[(mostly gone) XtC4UaLL [:xtc4uall]]

Seeing this on Windows 7, too.

Comment 2

[William Chen [:wchen]]

The reason this happens is because the XUL alerts use a <xul:label> for the text of the alert and it doesn't support newlines. This isn't a regression, it's been that way in the XUL alerts since hg 1 so I'm not sure this bug should be tracked. If we do want to allow new lines in alerts, then I think this bug would be better assigned to someone with more XUL experience.

Comment 3

[Reuben Morais [:reuben]]

(In reply to William Chen [:wchen] from comment #2)
> The reason this happens is because the XUL alerts use a <xul:label> for the
> text of the alert and it doesn't support newlines. This isn't a regression,
> it's been that way in the XUL alerts since hg 1 so I'm not sure this bug
> should be tracked. If we do want to allow new lines in alerts, then I think
> this bug would be better assigned to someone with more XUL experience.

This is a regression in notifications. I can reword the bug title, but the underlying problem is still the same: newlines used to work, and now they don't.

Can't we use something else instead of a <xul:label>?

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

I think we may just need to set the |whitespace: pre;| CSS style on the label.

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Actually, that should be |white-space: pre-wrap;|

Comment 6

[Reuben Morais [:reuben]]

Attachment: Add white-space: pre-wrap; to alert text

<Unfocused> such a beautiful bromance

Why don't you play a part in this heartwarming story?

Comment 7

[Reuben Morais [:reuben]]

\o/

https://hg.mozilla.org/integration/mozilla-inbound/rev/0134d330663f

Comment 8

[Reuben Morais [:reuben]]

Oops, that file uses 2 space indentation.

https://hg.mozilla.org/integration/mozilla-inbound/rev/d9cf9e32f267

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/0134d330663f
https://hg.mozilla.org/mozilla-central/rev/d9cf9e32f267

Comment 10

[Alex Keybl [:akeybl]]

Would you mind requesting uplift to Aurora/Beta to make sure this is resolved in FF23, preventing a regression?

Comment 11

[Reuben Morais [:reuben]]

Comment on attachment 753515 [details] [diff] [review]
Add white-space: pre-wrap; to alert text

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 782211
User impact if declined: multi-line notifications shown in a single line
Testing completed (on m-c, etc.): Landed on m-c a week ago, no problems have surfaced
Risk to taking this patch (and alternatives if risky): None
String or IDL/UUID changes made by this patch: None

Comment 12

[Reuben Morais [:reuben]]

Thanks!

https://hg.mozilla.org/releases/mozilla-aurora/rev/50125a7e87f7
https://hg.mozilla.org/releases/mozilla-beta/rev/ef532a492aa4

Comment 13

[Cornel Ionce [:noni] [Hubs QA]]

Is there anything QA can do here?

Comment 15

[Ioana (away)]

Can you please give me some examples of notifications with which I could reproduce/verify this bug?

Comment 16

[Reuben Morais [:reuben]]

(In reply to Ioana Budnar, QA [:ioana] from comment #15)
> Can you please give me some examples of notifications with which I could
> reproduce/verify this bug?

Open the browser console (Ctrl/Cmd+Shift+J), paste the text from comment 0 and press enter:
  Cc["@mozilla.org/alerts-service;1"].getService(Ci.nsIAlertsService).showAlertNotification(null, "Title", "Foo\nBar");

You should see three separate lines of text in the notification: "Title", "Foo" and "Bar".

Comment 17

[Ioana (away)]

I thought there was some other way. I tried with the Error Console and Web Console and I got "ReferenceError: Cc is not defined".

Firefox 23 doesn't have the Browser Console.

Comment 18

[Reuben Morais [:reuben]]

You can also flip the "devtools.chrome.enabled" pref to true, and then run that code in Scratchpad, with the Browser environment.

Comment 19

[Reuben Morais [:reuben]]

Hm, WebNotifications landed in 22, so alternatively you can also do this in the page console or scratchpad:

Notification.requestPermission();
*Allow notifications using the doorhanger popup*
new Notification("Title", { body: "Foo\nBar" });

Comment 20

[Ioana (away)]

(In reply to Reuben Morais [:reuben] from comment #19)
> Hm, WebNotifications landed in 22, so alternatively you can also do this in
> the page console or scratchpad:
> 
> Notification.requestPermission();
> *Allow notifications using the doorhanger popup*
> new Notification("Title", { body: "Foo\nBar" });

Thanks for all the help Reuben. I verified this as fixed on Firefox 23 beta 10 (20130729175331), Ubuntu 13.04 32bit and Windows 7 64bit.

I tried to verify it on Mac OSX 10.7, but there's no notification shown there for some reason. Is this a known issue, or somehow designed to work differently on Mac?

Comment 21

[Reuben Morais [:reuben]]

(In reply to Ioana Budnar, QA [:ioana] from comment #20)
> I tried to verify it on Mac OSX 10.7, but there's no notification shown
> there for some reason. Is this a known issue, or somehow designed to work
> differently on Mac?

I don't think so. Can you check if you have Growl installed/runnning, and in that case, if closing it makes any difference? If no notifications show up, please file a new bug under Core :: Widget: Cocoa. Thanks for the help!

Comment 22

[Ioana (away)]

There was some Growl framework that came with the OS, but no app/pane/options to work with it. I didn't find anything related to it in the running processes list either. => Notifications didn't work at all.

Installed Growl 1.2.2 and let it run => Notifications didn't work.

Stopped Growl => Some notifications were displayed (new Notification("Title", { body: "Foo\nBar" });), but very few.

Removed Growl => Notifications didn't work.

Reinstalled Growl => Notification didn't work with Growl on, nor off.

Same behavior on Firefox 23 RC, 24 Aurora and 25 Nightly.

What are the expected results here?

Comment 23

[u279076]

I don't believe this bug was specific to Growl but the native OS notification on all platforms. Comment 0 has a fairly clear testcase. If that doesn't work then please need-info Rueben.

Note that testing with Growl is a moot point since we dropped support for it in Firefox 22 (bug 777409).

Comment 24

[Ioana (away)]

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #23)
> I don't believe this bug was specific to Growl but the native OS
> notification on all platforms. Comment 0 has a fairly clear testcase. If
> that doesn't work then please need-info Rueben.
> 
> Note that testing with Growl is a moot point since we dropped support for it
> in Firefox 22 (bug 777409).

Comment 22 was an answer to Reuben's previous comment, where I was asked to also check how Growl affects this. I am waiting for him to confirm what the issue is here.

Comment 25

[Reuben Morais [:reuben]]

I asked you to test if Growl affects WebNotifications because that would be a Widget: Cocoa bug, given bug 777409. I'll look into it and file bugs if necessary. Thanks Ioana!