Closed Bug 854035 Opened 9 years ago Closed 9 years ago

PopupNotification should reset mainAction label when it's removed

Categories

(Toolkit :: General, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: Dolske, Assigned: Dolske)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch Patch v.1 (obsolete) — Splinter Review
Found testing bug 737100. The door hanger there follows the typical allow/always/never, but with the twist that if the underlying permission is already "always", we show a buttonless prompt as a reminder.

That works, unless you first show the prompt with a button and then show it again in the same session without the button.

EG (with v10 of patch there)
0) load http://mozilla.pettay.fi/moztests/pointerlock/pointerlock.html
1) click "lock"
2) get the allow/always/never prompt, select "always"
3) Hit escape, click "lock again"
4) get the reminder prompt, but there is an unexpected "allow" button

If you open a new window, load that page, and click "lock" you get the expected buttonless prompt.

Trivial fix. xul.css control the button's display via:

206 .popup-notification-menubutton:not([label]) {
207   display: none;
208 }
Attachment #728487 - Flags: review?(mnoorenberghe+bmo)
Comment on attachment 728487 [details] [diff] [review]
Patch v.1

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

::: toolkit/content/PopupNotifications.jsm
@@ +473,5 @@
>          popupnotification.setAttribute("buttoncommand", "PopupNotifications._onButtonCommand(event);");
>          popupnotification.setAttribute("menucommand", "PopupNotifications._onMenuCommand(event);");
>          popupnotification.setAttribute("closeitemcommand", "PopupNotifications._dismiss();event.stopPropagation();");
> +      } else {
> +        popupnotification.removeAttribute("buttonlabel");

I may be more consistent to remove the attributes that we don't set when there is no mainAction.
Attachment #728487 - Flags: review?(mnoorenberghe+bmo) → review+
Attached patch Patch v.2Splinter Review
Assignee: nobody → dolske
Attachment #728487 - Attachment is obsolete: true
Blocks: 737100
https://hg.mozilla.org/mozilla-central/rev/3536b7a3185b
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.