Closed
Bug 1225563
Opened 5 years ago
Closed 5 years ago
Log error in JS that Doorhangers only support one positive and one negative button if invalid buttons are provided
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox47 fixed)
RESOLVED
FIXED
Firefox 47
Tracking | Status | |
---|---|---|
firefox47 | --- | fixed |
People
(Reporter: azrdev, Unassigned, Mentored)
Details
(Whiteboard: [lang=js][good first bug])
Attachments
(1 file, 2 obsolete files)
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:42.0) Gecko/20100101 Firefox/42.0 Build ID: 20151103081951 Steps to reproduce: In an addon, call NativeWindow.doorhanger.show("label", "const id", [button1, button2], ...); where the buttons are objects with `label` and `callback` members, but *not* a `positive:true`. As per <https://developer.mozilla.org/en-US/Add-ons/Firefox_for_Android/API/NativeWindow/doorhanger#show%28%29>, only one of the Buttons is shown. Actual results: There is no indication why the button(s) have been dropped from display. Expected results: A log message would be useful.
Updated•5 years ago
|
Mentor: margaret.leibovic
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [lang=js][good first bug]
Updated•5 years ago
|
Mentor: liuche
Comment 1•5 years ago
|
||
We do log an error, but it goes to Logcat in Android. Since this mainly affects Addon authors, this notification should be in JS. http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/DoorHangerPopup.java#131
Summary: NativeWindow.doorhanger.show() with multiple buttons silently discards all but one → Log error in JS that Doorhangers only support one positive and one negative button if invalid buttons are provided
Comment 2•5 years ago
|
||
Updating these links: In the Java code, we log an error if there are more than two buttons. http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/DoorHangerPopup.java#133 We should do this same check in JS when sending the message to create doorhanger buttons, and log an error if there are more than two buttons. http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#2300
Comment 4•5 years ago
|
||
Comment on attachment 8719689 [details] [diff] [review] Log error in JS that Doorhangers only support one positive and one negative button if invalid buttons are provided Review of attachment 8719689 [details] [diff] [review]: ----------------------------------------------------------------- One nit, but otherwise looks good! If you'll post an updated patch, we'll mark it checkin-needed. (That's a Bugzilla keyword; I'm not sure you have the rights to set it just yet.) ::: mobile/android/chrome/content/browser.js @@ +2288,5 @@ > if (aButtons == null) { > aButtons = []; > } > > + if(aButtons.length > 2) { nit: space between if and (.
Attachment #8719689 -
Flags: review+
Attachment #8719689 -
Attachment is obsolete: true
Attachment #8722662 -
Flags: review?(nalexander)
Comment 7•5 years ago
|
||
Comment on attachment 8722662 [details] [diff] [review] Log error in JS that Doorhangers only support one positive and one negative button if invalid buttons are provided Review of attachment 8722662 [details] [diff] [review]: ----------------------------------------------------------------- Looks good.
Attachment #8722662 -
Flags: review?(nalexander) → review+
Keywords: checkin-needed
Attachment #8720973 -
Attachment is obsolete: true
Comment 9•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e38a16b7d37e
Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Assignee | ||
Updated•1 month ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•