Skip to content
This repository has been archived by the owner on Sep 9, 2022. It is now read-only.

[Firefox] Popup blocker incorrectly closes user clicked window #507

Closed
harshanvn opened this issue Jan 15, 2015 · 30 comments
Closed

[Firefox] Popup blocker incorrectly closes user clicked window #507

harshanvn opened this issue Jan 15, 2015 · 30 comments

Comments

@harshanvn
Copy link

Hi Gorhill,

Try clicking 2nd link "Watch this link!" (i.e., putlocker | vodu ) at below link -
http://watchseries.lt/episode/Forever_s1_e13.html

Then click "click here to play" link. Instead of opening the link it gets automatically closed. However if you open "click here to play" link normally, it works fine.

Using 8.5.4.beta0 version..

@gorhill
Copy link
Contributor

gorhill commented Jan 15, 2015

Can't reproduce, worked fine from here.

Edit: I am not sure what you mean by "open ... normally"... Isn't clicking on a link the normal way to open it?

@harshanvn
Copy link
Author

hmm. Am able to reproduce constantly. Is there any way to do screen record. Perhaps i can upload the video..

opening normally, i meant was to copy the link location (right click), and open a new tab, paste it. This way it works fine. But not, if click the link directly..

Am on windows. Maybe some from windows platform can test and confirm

@gorhill
Copy link
Contributor

gorhill commented Jan 15, 2015

No need to record, I believe you. We just need somebody else to reproduce so that we can identify a scenario. The OS could be a factor I suppose.

@Deathamns
Copy link
Contributor

After you installed the version from the releases section, restart your browser, and try then.

BTW, Nightly on Win, worked fine.

@harshanvn
Copy link
Author

Yup, I did that. And the issue persists.
Also, if i manually disable and enable the extension it works first time. And issue comes from 2nd time onwards.

Also, i see 0.8.5.3 on popUI and 0.8.5.4 in addon manager. I dont think this matters.

@rodalpho
Copy link

Worked fine for me with the release version on Fx 35.0. Page was not closed.

@gorhill
Copy link
Contributor

gorhill commented Jan 15, 2015

Yes, I forgot to mention I am using FF 35.

@Deathamns
Copy link
Contributor

@harshanvn Default settings?

@harshanvn
Copy link
Author

Ahh, i should have said the second link in that..

Then click second "click here to play" link. Instead of opening the link it gets automatically closed. However if you open "click here to play" link normally, it works fine.

Edit: ignore above statement.

When you are clicking "Watch this link" at the following url http://watchseries.lt/episode/Forever_s1_e13.html.
Are you clicking second button "watch this link" right?

Clicking on other "watch this link" works fine..

@harshanvn
Copy link
Author

Am on v35

@Deathamns I enabled inline script, 3p script and iframe filtering. And all the check boxes in the settings window.

@gorhill
Copy link
Contributor

gorhill commented Jan 15, 2015

@harshanvn What other add-ons, if any?

@harshanvn
Copy link
Author

i have noscript, but it is already disabled.

@harshanvn
Copy link
Author

Am going to uninstall and install and see if it makes a difference. (backed my settings :) )

@harshanvn
Copy link
Author

Ok I figured it out.

I have this filter, in my filter tab ( i should have looked at it earlier, my bad ) - ||vodu.ch^$popup which is causing the issue.

I dont think it should close the user clicked link...

@gorhill
Copy link
Contributor

gorhill commented Jan 15, 2015

I dont think it should close the user clicked link...

I confirm same filter in Chromium does not cause the tab to close.

@gorhill
Copy link
Contributor

gorhill commented Jan 15, 2015

Looks to me the tab is blocked by the code in onBeforeSendHeaders, not the code in vAPI.tabs.onPopup. (I forgot to add the logging and I see no popup log entry in the log never mind, can't have logging in onBeforeSendHeaders ).

@Deathamns
Copy link
Contributor

8th test on the list from #510.

@gorhill
Copy link
Contributor

gorhill commented Jan 28, 2015

8th test on the list from #510

What about it?

Isn't the primary issue here fixed?

If so then I guess a preliminary version can be submitted to Mozilla?

@Deathamns
Copy link
Contributor

The 8th was a case where I couldn't decide what should happen when clicking a link inside a pop-up.
As this issue shows, it's better not to close it.
The problem is that when a user navigates in a pop-up, it will still be considered as a newly opened pop-up, and it will be closed if it's on the block-list.

@gorhill
Copy link
Contributor

gorhill commented Jan 28, 2015

Looks to me that OP issue is fixed, despite #8 in test cases resulting in popup window closing. ABP closes also when we click in #8.

@Deathamns
Copy link
Contributor

It's not fixed. Don't forget to add the ||vodu.ch^$popup net-filter.

@gorhill
Copy link
Contributor

gorhill commented Jan 28, 2015

With ||vodu.ch^$popup I get same behavior with ABP.

Edit: given that the current behavior is same as ABP, it's not enough to hold onto submitting to Mozilla IMO.

@Deathamns
Copy link
Contributor

It seems like I was the one who forgot to add ||vodu.ch^$popup to ABP, because I was believing that there was no problem there, and this is why I wanted to fix that before sending it.

As for the solution for this, the first trusted user-interaction (on keydown and mousedown events) could be detected, and after that don't consider that tab/window as a pop-up.

@harshanvn
Copy link
Author

I was not aware that the same issue persists with abp. So, it should not be blocker for release.

We should definitely plan for a fix when time permitted. As the popup directive to the host site is the effective way to block current and future popups.

Also thinking a bit about, would it be a good idea to add pop up blocking to dynamic filtering? This way it will be easy to mitigate annoying pop up issues and need not to rely on filters.. Just my 2 cents.

@gorhill
Copy link
Contributor

gorhill commented Jan 28, 2015

the popup directive to the host site is the effective way to block current and future popups

Not from the filter you used. What you wanted is |http:$popup,domain=vodu.ch. (uBlock currently doesn't parse * as a lone placeholder for an entire URL, maybe it should..)

to add pop up blocking to dynamic filtering

I have been considering it a lot lately. However, this would have to be a feature available to non-advanced users as well (though internally this would use a dynamic filtering rule). So where I am at now is to figure a UI in the basic popup UI which does not results in the UI crossing the line from non-cluttered to cluttered. I don't need suggestions, I will figure it, just taking my time.

@Deathamns
Copy link
Contributor

Actually, it doesn't seem too complicated to implement my idea, so I'll make a try before sending it.

@harshanvn
Copy link
Author

I have been considering it a lot lately. However, this would have to be a feature available to non-advanced users as well

nice. And can't appreciate enough for your nice work on this extension (also @Deathamns , @chrisaljoudi for their contributions ). Really like it a lot.

@Deathamns
Copy link
Contributor

Test it ee5a023. It worked pretty well for me, but it's probably not a final solution.

@gorhill
Copy link
Contributor

gorhill commented Jan 28, 2015

Yep, worked fine here.

@gorhill
Copy link
Contributor

gorhill commented Feb 7, 2015

Can we close this?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants