Skip to content

Loading…

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

Closed
harshanvn opened this Issue · 30 comments

4 participants

@harshanvn

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

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

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

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

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

BTW, Nightly on Win, worked fine.

@harshanvn

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

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

@gorhill

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

@Deathamns

@harshanvn Default settings?

@harshanvn

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

Am on v35

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

@gorhill

@harshanvn What other add-ons, if any?

@harshanvn

i have noscript, but it is already disabled.

@harshanvn

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

@harshanvn

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

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

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

@gorhill

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

8th test on the list from #510.

@gorhill

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

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

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

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

@gorhill

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

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

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

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

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

@harshanvn

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

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

@gorhill

Yep, worked fine here.

@gorhill

Can we close this?

@Deathamns Deathamns closed this
@AlexVallat AlexVallat pushed a commit to AlexVallat/uBlock that referenced this issue
@gorhill gorhill this fixes #507 528354f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.