Closed Bug 862201 Opened 8 years ago Closed 5 years ago

Mixed conted doorhanger disappears when user selects "Keep Blocking"

Categories

(Core Graveyard :: Security: UI, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: Dolske, Unassigned)

References

(Blocks 1 open bug)

Details

1) Visit https://vine.co/v/bFd1IDuwiKu
2) Get mixed content blocker icon in url bar (bug 861753)
3) Click icon, get doorhanger, click Keep Blocking
4) doorhanger and icon (!) go away.

This is a bit surprising, since normally it sticks around because "Keep Blocking" doesn't actually do anything. I wanted to unblock the content, and had to reload the page to get the icon back.
This is where the popupnotification is set up: https://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#6444.  The "Keep Blocking" button is created in the "action" variable.  Justin, do you know how to keep the shield icon when the user clicks "Keep Blocking"?
(In reply to Tanvi Vyas [:tanvi] from comment #1)
> This is where the popupnotification is set up:
> https://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.
> js#6444.  The "Keep Blocking" button is created in the "action" variable. 
> Justin, do you know how to keep the shield icon when the user clicks "Keep
> Blocking"?
Flags: needinfo?(dolske)
(In reply to Tanvi Vyas [:tanvi] from comment #1)

> Justin, do you know how to keep the shield icon when the user clicks "Keep
> Blocking"?

Not offhand. You'll presumably need to give PopupNotifications some way of knowing a provided action is actually a NOP (ala Not Now).
Flags: needinfo?(dolske)
Duplicate of this bug: 902583
Summary: Mixed conted doorhanger disappears → Mixed conted doorhanger disappears when user selects "Keep Blocking"
Spoke to Matt and got some pointers:
We should add eventCallback function to the options of the mixed content blocker notification and listen for the "shown" event.  Then you can replace the buttoncommand attribute of the notification.

The default behaviour is PopupNotifications._onButtonCommand(event); so you can set it to an empty string to test.  You could probably replace it with PopupNotifications._dismiss(); to dismiss only

eventCallback options parameter: https://mxr.mozilla.org/mozilla-central/source/toolkit/modules/PopupNotifications.jsm?mark=218,227#205

Note that we can replace _onButtonCommand only after the shown event has triggered; doing it before would just result in it being rewritten because the DOM is reconstructed everytime the doorhanger is opened:
https://mxr.mozilla.org/mozilla-central/source/toolkit/modules/PopupNotifications.jsm?rev=8f24192cb2b2&mark=510#483
My other idea is to have PopupNotifications check the return type of the mainAction callback and change behaviour based on the value. (e.g. return false to not remove).

If we want to switch the callback to be async at some point it may make this idea more complicated.
"main" actions that do nothing are kind of a weird edge case, and we do want to move to async callbacks ideally (if it's not already too late), so I don't really like the return value idea.

It looks like http://hg.mozilla.org/mozilla-central/diff/22737775ef32/toolkit/modules/PopupNotifications.jsm#l1.12 added exactly what you need, though - a way to specify that the main action should act like "dismiss" rather than "remove".
AFACT, the "keep blocking" button was removed.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WORKSFORME
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.