847620 - Work - Tapping the browser content should hide the pop-up blocker bar

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: General, defect, P2)

Description

[Matt Brubeck (:mbrubeck)]

According to attachment 703515 [details], tapping the content area should dismiss the blocked-popup notification.

Asa - Is this behavior only for the pop-up blocker info app bar, or should it work the same for other info app bars (e.g. content permissions, password manager).

Note that desktop Firefox does *not* dismiss notification bars when clicking in content.  On desktop we do this only for doorhanger notifications (and we provide a way to bring back those notification by clicking on its "anchor" icon in the location bar, in case of accidental dismissal).

Comment 1

[Marco Mucci [:MarcoM]]

Hey Matt, will you need design work from Yuan or Stephen for this?

Comment 2

[Asa Dotzler [:asa]]

Only for the pop-up blocker info app bar because it is triggered without user interaction and I want it to go away as quickly as possible. IE seems to have their pop-up blocker on a timer so it automatically goes away after about 5 seconds. I think tap on content to dismiss is sufficiently transient  though.

(My stories for the others need updating. They should not be so transient.The info app bars for form saving, password saving, and geolocation, should each be sticky.)

Comment 3

[Matt Brubeck (:mbrubeck)]

Thanks, Asa!

(In reply to Marco Mucci [:MarcoM] from comment #1)
> Hey Matt, will you need design work from Yuan or Stephen for this?

No, I think I have everything.

Comment 4

[Matt Brubeck (:mbrubeck)]

Attachment: patch

Comment 5

[Rodrigo Silveira [:rsilveira] [:rodms]]

Comment on attachment 722788 [details] [diff] [review]
patch

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

Applied the patch and it works. Looks good.

::: browser/metro/base/content/browser.js
@@ +1225,5 @@
>   * Handler for blocked popups, triggered by DOMUpdatePageReport events in browser.xml
>   */
>  var PopupBlockerObserver = {
> +  init: function init() {
> +    Elements.browsers.addEventListener("mousedown", this, true);

Just wondering if there is a way to only hook this up after DOMPopupBlocked or something? If I understand correctly, this listener would run for every click.

Comment 6

[Matt Brubeck (:mbrubeck)]

(In reply to Rodrigo Silveira [:rsilveira] from comment #5)
> Just wondering if there is a way to only hook this up after DOMPopupBlocked
> or something? If I understand correctly, this listener would run for every
> click.

Yeah, I started implementing it that way but it got a bit complex: there may be multiple popup notifications active at once in different tabs, so you need to keep track of when they are added and removed, to decide when to remove the listener again.

Since we already have JavaScript "mousedown" listeners in several places, I decided this was premature optimization; the added complexity is only worth it if we discover this listener causes a measurable performance problem.

Comment 7

[Matt Brubeck (:mbrubeck)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/1d65c8f296d9

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1d65c8f296d9