1297707 - TypeError prevents alert dialog to display icon

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Stephen A Pohl [:spohl]]

Run this code in the browser console:

> setTimeout(() => Services.prompt.alert(window, "Hi", null), 5000);

Expected:
An alert dialog appears with a yellow triangle and exclamation point as icon.

Actual:
No icon is displayed in the dialog and the following error is printed in the browser console:
TypeError: this.args.text is null[Learn More]CommonDialog.jsm:132:13

CommonDialog.prototype.onLoadresource://gre/modules/CommonDialog.jsm:132:13commonDialogOnLoadchrome://global/content/commonDialog.js:52:5<anonymous>chrome://global/content/commonDialog.xul:27:7openModalWindowfile:///Users/Stephen/Documents/objdir-ff-release/dist/Nightly.app/Contents/Resources/components/nsPrompter.js:364:5ModalPrompter.prototype.openPromptfile:///Users/Stephen/Documents/objdir-ff-release/dist/Nightly.app/Contents/Resources/components/nsPrompter.js:554:9ModalPrompter.prototype.alertfile:///Users/Stephen/Documents/objdir-ff-release/dist/Nightly.app/Contents/Resources/components/nsPrompter.js:603:9Prompter.prototype.alertfile:///Users/Stephen/Documents/objdir-ff-release/dist/Nightly.app/Contents/Resources/components/nsPrompter.js:59:9<anonymous>debugger eval code

Comment 1

[Stephen A Pohl [:spohl]]

Attachment: Patch

Felipe, I see you've reviewed some code in this file but please feel free to forward the review if needed. Thank you!

Comment 2

[Stephen A Pohl [:spohl]]

Attachment: Patch

Cleaned this up a bit.

Comment 3

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8784371 [details] [diff] [review]
Patch

Review of attachment 8784371 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, but I prefer this first version. This way the text node will always be created and the DOM structure will be the same.

Is this just clean-up, or is there any motivation for it? When would someone call a prompt without text?

Comment 4

[Stephen A Pohl [:spohl]]

So, the following code (from comment 0) actually triggers this, and we then fail to show the icon:

> setTimeout(() => Services.prompt.alert(window, "Hi", null), 5000);

I thought "Hi" was the text in question here, but it doesn't appear to be or we wouldn't hit this.

Comment 5

[:Felipe Gomes (needinfo for replies!)]

Oh, by looking at http://searchfox.org/mozilla-central/source/embedding/components/windowwatcher/nsIPromptService.idl#53
the text is the third parameter. "Hi" is the title. They appear visually similar :)

Comment 6

[Stephen A Pohl [:spohl]]

Good point, thanks for clearing that up! :-) I also went ahead and confirmed that we don't run into this same issue if the title is null but the text is a valid string; we already handle that situation properly.

Comment 7

[Stephen A Pohl [:spohl]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a5657c5416cec71c7d589a7d67a2b7ceb2bad635
Bug 1297707: Show icons in alert dialogs when text is null. r=felipe

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a5657c5416ce

Comment 9

[Azmina]

I have reproduced this bug with Nightly 51.0a1 (2016-08-24)on Windows 8.1 (64 Bit!).

This bug's fix is verified on Latest Nightly 51.0a1.
Build ID : 20160831030224
User Agent : Mozilla/5.0 (Windows NT 6.3; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0

[bugday-20160831]

Comment 10

[Stephen A Pohl [:spohl]]

Thank you!