Closed Bug 1297707 Opened 3 years ago Closed 3 years ago

TypeError prevents alert dialog to display icon

Categories

(Firefox :: General, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 51
Tracking Status
firefox51 --- verified

People

(Reporter: spohl, Assigned: spohl)

Details

Attachments

(1 file, 1 obsolete file)

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
Assignee: nobody → spohl.mozilla.bugs
Attached patch PatchSplinter Review
Felipe, I see you've reviewed some code in this file but please feel free to forward the review if needed. Thank you!
Attachment #8784371 - Flags: review?(felipc)
Attached patch Patch (obsolete) — Splinter Review
Cleaned this up a bit.
Attachment #8784371 - Attachment is obsolete: true
Attachment #8784371 - Flags: review?(felipc)
Attachment #8784374 - Flags: review?(felipc)
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?
Attachment #8784371 - Flags: review+
Attachment #8784371 - Attachment is obsolete: false
Attachment #8784374 - Flags: review?(felipc)
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.
Flags: needinfo?(felipc)
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 :)
Flags: needinfo?(felipc)
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.
https://hg.mozilla.org/mozilla-central/rev/a5657c5416ce
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
Attachment #8784374 - Attachment is obsolete: true
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]
Thank you!
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.