Closed Bug 847620 Opened 9 years ago Closed 9 years ago

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

Categories

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

All
Windows 8.1
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mbrubeck, Assigned: mbrubeck)

References

Details

(Whiteboard: feature=work)

Attachments

(1 file)

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).
Flags: needinfo?(asa)
Keywords: uiwanted
Hey Matt, will you need design work from Yuan or Stephen for this?
Priority: -- → P2
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.)
Flags: needinfo?(asa)
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.
Keywords: uiwanted
Attached patch patchSplinter Review
Attachment #722788 - Flags: review?(sfoster)
Attachment #722788 - Flags: review?(sfoster) → review?(rsilveira)
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.
Attachment #722788 - Flags: review?(rsilveira) → review+
(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.
https://hg.mozilla.org/mozilla-central/rev/1d65c8f296d9
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Depends on: 850398
No longer depends on: 850398
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.