Skip to content

Loading…

[Firefox on Android] "Open Link in New Tab" gets blocked in Behind the Scene with "* * * block" #1001

Closed
my-password-is-password opened this Issue · 34 comments

4 participants

@my-password-is-password

Steps to Reproduce

  1. Default settings with changes to
    • "Whitelist" , remove or comment out behind-the-scene
    • "My Rules" , set it to * * * block
  2. Go to any page, long press any link and pick "Open Link in New Tab"

Symptoms

The link opens in a new tab but its completely blank. If you check the request log for "Behind the scene" it shows that the page gets blocked with * * * block.

Video:
http://uux3.altervista.org/uBlockFFMobileNewTabIssue.webm

Using 0.9.0.1-dev.4

@gorhill gorhill added a commit that closed this issue
@gorhill gorhill this fixes #1001 6dd4a24
@gorhill gorhill closed this in 6dd4a24
@gorhill gorhill added fixing and removed fixing labels
@gorhill

Are you building from Github repo or do you need a dev release? If this can tested before a release that would be good (I can't test the fix, I have no mobile device).

@my-password-is-password

For firefox I use the dev release you put up.

@gorhill

Ok, I am creating a new dev release.

@my-password-is-password

Its fixed with 0.9.0.1-dev.5. The link loads in the new tab. Thanks

@my-password-is-password

@gorhill The loading of the link is fixed but its not blocking. I just tried going to a site with ads and the the ads are showing.

@gorhill

You mean when they are pulled from the behind-the-scene tab?

@my-password-is-password

Yeah, on the link opened with "Open Link in New Tab". There is no blocking at all.

Video:
http://uux3.altervista.org/uBlockFFMobileNewTabIssueDev0.9.0.1-dev.5.webm

@Deathamns

Probably only the 'main_frame' went behind-the-scene, the rest had tabIds.
Also, why does the fix work? In the Firefox code parentFrameId is not even set for behind-the-scene requests.

@gorhill

Ok I understand what is happening: the root document is pulled from behind-the-scene, which means it is not and cannot be bound to a specific tab id. So all subsequent network requests from within that page can't find the page store, and as a consequence no filtering is taking place.

So ideally, when the main document is requested, the platform-dependent code should try to find the tab id information. If that is not possible, then I will have to think of something else. Maybe the tab id information is available a bit later, like when onHeadersReceived is called. @AlexVallat needs to look into this.

@gorhill

I created a release just for you, take it and tell me when you have it, I will remove it asap. Just a tentative fix, tell me if this works.

@my-password-is-password

Ok i got it and going to test it now.

@my-password-is-password

@gorhill Doesn't fix it. Behaves just like 0.9.0.1-dev.5

@gorhill

Ok so we need something else.. I think a new issue should be opened for this, really it is different than the original one.

@gorhill

It would be useful to have a mode in the request logger to show the network requests for all existing tabs. Like an "All" entry in the dropdown list. I would like to know what is the first network request received from the new tab.

@gorhill

@my-password-is-password if you force refresh the tab after it opens, can you show me what the request log report for that tab?

@gorhill

Works after a refresh.

Yes, expected. I was just wondering what are the first network requests in the log after a refresh. But not really important after more thinking.

Anyway, I think uBlock needs to fall back into a least-worse mode if it can't find a valid page store for a valid tab id. "Least-worst" meaning the context may not be the right one, but at least some filtering can take place. I could even remember the URL of the last root doc encountered, and use this for when no page store can be found for a network request, that would be a best guess approach, which I believe would solve the problem here, and not have any effect for how uBlock's behave for other platforms where there are no issues.

@Deathamns

Probably proper onNavigation implementation for Fennec would fix this.

@my-password-is-password

@gorhill I can't open the request log for that tab before the refresh because the little eye is not visible in the popup.

@gorhill

@my-password-is-password I know... never mind.

@gorhill

@Deathamns

Also, why does the fix work? In the Firefox code parentFrameId is not even set for behind-the-scene requests.

So what is the explanation?

I will try to create a generic workaround for the situation here, and for this I really need the root document to have its parent id set to -1 even for behind the scene request. If eventually the platform-specific code get fixed, this will be great, but still, I believe a generic workaround for the current case is needed: uBlock should never fall back into a nothing-filtered mode.

@gorhill gorhill reopened this
@gorhill

Going to try to provide a fix for the reported issue and for the subsequent issue of no filtering occurring for the newly opened tab.

@gorhill gorhill closed this in 4a76b6f
@gorhill

@my-password-is-password Can you fetch/test the special release to see if this fixes all issues reported here?

@Deathamns

Side question: can the main_frame have anything else than -1 for parentFrameId?

@my-password-is-password

@gorhill Ok, downloaded the special release and going to test now.

@gorhill

Meh, I made a "oops" mistake...

Re-released. My bad, too eager to have the fix tested.

@my-password-is-password

Yeah, Its not blocking and refreshing gets blocked in behind the scene again by * * * block.

@gorhill

Side question: can the main_frame have anything else than -1 for parentFrameId?

I really do not know, it's not spelled out explicitly in the Chrome API doc if this is the case.

@gorhill

@my-password-is-password new release available, I made a silly mistake.

@my-password-is-password

@gorhill Ok downloaded and install. Going to test again.

@gorhill

@Deathamns Well it does say:

Request types such as main_frame (a document that is loaded for a top-level frame)

@gorhill

@Deathamns Ok I will trust the Chrome API doc, and I won't test for parentFrameId anymore for when the request is of type main_frame. So no change needed on your side.

@AlexVallat AlexVallat added a commit to AlexVallat/uBlock that referenced this issue
@AlexVallat AlexVallat Testing alternative tab ID for Fennec (issue #1001) f0e4c42
@AlexVallat
Collaborator

Arriving a little late to this party, but I thought I might as well note my findings. The underlying problem in Fennec seems to be that in vapi-background.js the shouldLoadListener handler is receiving the shouldLoad message from the newly opened tab before the new browser actually has a tab created for it. So when it tries to look up the tab by vAPI.tabs.getTabId(e.target) it always gets -1, because that tab doesn't yet exist.

There are various workarounds that can be made to deal with background requests and root frames and so on, but it doesn't change the basic fact that these requests are coming in as "background" and not as belonging to a new tab. The workaround implemented, as I understand it, relies on requests for elements of a page immediately following the request for the root document of that page - as your comment says, a guess, but not an unreasonable one. It would probably be quite a strange combination of server response times and opening multiple tabs to trip this up.

The alternative is to use something other than tab.id as the identity to key the pageStore on - something that's accessible on the browser, because the tab doesn't exist yet. There is browser.loadContext.DOMWindowID which looks promising. I've tried this in a branch: gorhill/uBlock@master...AlexVallat:alternateTabId and it seems to work OK. When running the test to reproduce this issue, the new link opened in a new tab is correctly assigned its own pageStore, to which the other content on that page is also assigned. It does not show up as behind the scenes.

@AlexVallat AlexVallat added a commit to AlexVallat/uBlock that referenced this issue
@AlexVallat AlexVallat Testing alternative tab ID for Fennec (issue #1001) 0554010
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.