AdguardTeam / PopupBlocker

Popup blocking userscript
GNU Lesser General Public License v3.0
337 stars 31 forks source link

Pop-up blocker warning alerts #13

Closed ameshkov closed 7 years ago

ameshkov commented 7 years ago

Here is the task for the alerts layout: https://github.com/AdguardTeam/PopupBlocker/issues/12

Some general notes on the task:

  1. Alerts should be shown inside of a dynamically created iframe (look at how it's done in Assistant) in order to not mess with the websites' own styles.
  2. Instead of generic blocking, we can now simply suppress all the pop-ups and show the alert. The only exception should be the obvious pop-ups.
  3. For every new blocked pop-up, you should show a new alert. Pop-up is new if the URL is different.
  4. The pop-up blocker text will be translated via oneskyapp, so you should not hardcode strings.

Alert state 1

popup blocker extension - page moqups 2017-08-10 12-43-49

Alert is shown when you block an attempt to open a window.

Alert state 2

After some time alert collapses into state 2: popup blocker extension - page c2 b7 moqups f0 9f 94 8a 2017-08-10 13-00-55

Click on the "pop-up" link expands it back to the state 1.

Also, if pop-up is detected by a "generic" algorithm from the first version of an extension, we should show alert in the collapsed state right away.

How alerts are stack

Basically, they are located one under one: https://monosnap.com/file/PFeTq9dLBERo0DYPbBeadRIcuqg11U

There should be no more than 4 alerts. When fifth emerges, remove the oldest one.

Alert lifecycle

2 seconds in the state 1. 5 seconds in the state 2. Disappears after that.

ameshkov commented 7 years ago

Regarding showing the stack trace, it won't help casual users, so there's no need to put it to the regular UI. However, it makes sense to add a "Stack trace" link to the dev and beta builds.

ameshkov commented 7 years ago

Also, in light of this change, I am no more sure we need to merge the #11. It seems that we can easily cover it with this change in a more user-friendly manner and less false-positive blockings.

theseanl commented 7 years ago

What made you think that #11 will cause more false-positives?

ameshkov commented 7 years ago

More complicated -> more false-positives

ameshkov commented 7 years ago

@seanl-adg added one more thing to the description:

Also, if pop-up is detected by a "generic" algorithm from the first version of an extension, we should show alert in the collapsed state right away.

theseanl commented 7 years ago

I need a description of the following:

ameshkov commented 7 years ago

Tested on http://code.ptcong.com/demos/bjp/demo.html

Issues

  1. State1->State2 transition should not be happening when user's mouse is over alert window;
  2. Click on the alert window on that demo page triggers pop-up second time;
  3. We need to have a JS confirmation dialog when the user clicks "Allow..." buttons;
  4. Increase state 1 lifetime to 3 seconds plz
ameshkov commented 7 years ago

Answers

When an 'Always allow example.org' is clicked, how are we going to notify that a domain is added to whitelist?

With a confirmation dialog it would be pretty clear. However, we should also reload the page (reload without cache).

How are we going to allow users to manage whitelisted domains, domains to use strict blocking mode, etc?

Will be discussed in another issue.

How can users enable strict blocking mode for a website?

This also should be discussed in another issue.

theseanl commented 7 years ago
  1. Click on the alert window on that demo page triggers pop-up second time;

That's legit, that's how BetterJsPop works. It adds its own masks on each of the document's iframes, including dynamically generated ones, and trigger popup when a user clicks on it.

ameshkov commented 7 years ago

Testing it on http://oploverz.in

  1. Click does not expand the alert back: https://monosnap.com/file/nraX5KMYZH0ZecC7GPvRt1hjYeunmI

  2. http:// prefix is redundant: https://monosnap.com/file/6gqO88vcxH77n4jbwd7bmZedTzZ7bd