Please support Firefox for Android

[stefanocaselli]

Hi, please support Firefox for Android (adblock plus works on firefox for android) thanks. PS I can help with testing if needed.

[gorhill]

vAPI.tabs.get is asynchronous, this means there absolutely no guarantee that the call to vAPI.tabs.onNavigation will be made before the next request(s) have to be processed. Best is this is handled in the Firefox-specific code, when there is no need to make any asynchronous call, and to call directly vAPI.tabs.onNavigation when all the information is available, i.e. when:

• this is a MAINFRAME
• it is the root one

This is it.

[gorhill]

Surely @Deathamns could provide the best insight as to where that Fennec-specific code should go ideally.

[AlexVallat]

tabs.get is async in Chrome? Hmm. Having a look at the firefox implementation of it, it does seem rather heavy-weight too, if all you want to do is get the URL from a tabId. Might be an argument there for a simpler getTabUrl(tabId) function to exist...

Anyway, if you really want to stick with the navigation event-based approach for other browsers then I guess this can be done in firefox-specific code, but without modifying traffic.js it would be a bit inefficient, duplicating a bunch of code from onBeforeRequest which would then run twice.

For your suggestion of calling onNavigation when receiving the request for the root main_frame, surely this will then run into the exact same #516 errors we are trying to avoid? Instead, it must call onNavigation for requests which are not the main_frame, when the page that they are being called for is not the current pageStore.pageURL.

So, to do it only in vapi-background, what we would have to do is copy and paste the code in the traffic.js onBackground function up to pageStore = µb.pageStoreFromTabId(tabId);, then check if the pageURL is different and call onNavigation if it is. Then do the onBeforeRequest.callback call and let it do all the same pageStore looking up again.

[gorhill]

simpler getTabUrl(tabId)

Everything in Chromium API is async, except for onBeforeRequest, for obvious reasons. However simpler you can craft it, it will still be async.

duplicating a bunch of code from onBeforeRequest which would then run twice

It is mostly a noop if the tab is already bound, notice the code:

• the binding occurs only for top main frame network request (i.e. once per web page), and
• the binding occurs if there is no binding found

So "a bit inefficient" is a complete non-issue.

[AlexVallat]

Everything in Chromium API is async

Wow, that's got to be a pain to work with.

OK, so in that case my proposed addition at https://github.com/gorhill/uBlock/blob/master/platform/firefox/vapi-background.js#L971 would be:

        if (details.frameId === 0 && // Only items loaded from top level frame, not iframes
            !(type === 'main_frame' && details.parentFrameId === -1)) { // Not main frame itself, only items loaded from it
            // Ensure the page store is associated with the correct main frame URL
            var mainFrameUrl = channel.loadInfo && channel.loadInfo.loadingDocument && channel.loadInfo.loadingDocument.URL;
            if (mainFrameUrl) {
                var pageStore = µBlock.pageStoreFromTabId(details.tabId);
                if (pageStore && µBlock.normalizePageURL(details.tabId, mainFrameUrl) !== pageStore.pageURL) {
                    vAPI.tabs.onNavigation({
                        frameId: details.frameId,
                        tabId: details.tabId,
                        url: mainFrameUrl,
                    });
                }
            }
        }

@Deathamns: could you take a quick look offer any comment too?

[gorhill]

// Not main frame itself, only items loaded from it

You are losing me.

Why not calling vAPI.tabs.onNavigation only when the request for the root frame is being processed? What you propose add an overhead to every request, instead of just that of the root frame.

[AlexVallat]

Why not calling vAPI.tabs.onNavigation only when the request for the root frame is being processed? What you propose add an overhead to every request, instead of just that of the root frame.

Because of #516, which was your whole reason for not calling pageStore.reuse on main_frame in the first place, wasn't it? In traffic.js, you specifically check for the root frame being processed, and specifically do not update the URL of the pageStore, by passing 'beforeRequest' as the context to bindTabToPageStats.

So, as I understand it, the original logic was:

• Root frame comes in, pageStore URL is updated to that page.

Then, as reported by #516, it is possible that a root frame comes in that should not result in the pageStore URL being updated, notably if you are downloading a file from the page. The logic was then changed to:

• Navigation event occurs, pageStore URL is updated to that page.

This is causing problems for Fennec because there is no navigation event that occurs before every possible content request for that page. Notably, the amazonaws script at motherjones.com. This means that for that request, the pageStore URL is wrong, as it wasn't updated by the root frame coming in (original logic) or by the navigation event (#516 revised logic)

My proposal is:

• Content request comes in, pageStore URL is updated to the URL of the main frame that the request belongs to (if it isn't already correct).

That means you don't need to rely on a navigation event occurring in the right order with respect to the content event, and you don't need to worry about file downloads or other main frame content requests.

I agree that it adds an overhead to every request, which is why I was quite keen to minimise it by using the pageStore that is already being looked up for every request by traffic.js anyway. Apart from that pageStore lookup, the additional overhead is a handful of direct property accesses, a normalizePageURL call and a string comparison (of the URL).

[Deathamns]

@AlexVallat I didn't really follow this thread, and I'm not in the mood to read it back, so what is the problem exactly that needs to be solved? But looking at it, this doesn't seem right for e10s: channel.loadInfo && channel.loadInfo.loadingDocument && channel.loadInfo.loadingDocument.URL;

[AlexVallat]

@Deathamns

what is the problem exactly that needs to be solved?

Fennec (Firefox for Android) does not have addTabsProgressListener. It does not have any navigation event that can be used to call vAPI.tabs.onNavigation that will always occur before any httpObserver.handleRequest for content on that page. Therefore, any page content that is loaded before the first available navigation event (confirmed to be DOMTitleChanged by @chengsun) will not be evaluated in the context of the correct page URL. Rules with $domain components will therefore be incorrectly applied.

[Deathamns]

Simply send a synchronous request for every main_frame, and listen for the pushState, replaceState in the top document.

[gorhill]

@AlexVallat

Just to give some perspective on #516... this bug is far from a showstopper: uBlock had been used for months before this was reported, and the problematic behavior occurs only when a user tries to download a document.

So re-enabling this bug should not be a showstopper for Fennec users, it was not for most users for months.

This means the simplest fix can be used for now. I would take care of this with this simple solution which is just a trivial change if I had a mobile device, but I unfortunately I don't. Longer term of course, trying to find a more permanent solution, but short term, have Fennec users access to uBlock, surely this is more important than that corner case.

[gorhill]

So for now I just copied wholly vapi-background.js from Firefox port into platform/fennec and added the line to call vAPI.tabs.onNavigate when a root frame is processed. There is a script ./tools/make-fennec.sh all to build a package. The port should suffer issue #516 for now, but this issue is magnitude less worst than not having a functional blocker at all. If someone can try this.

[AlexVallat]

@gorhill There are a lot of other fixes needed for a Fennec port, @chengsun has done all the work in https://github.com/chengsun/uBlock/tree/fennec and was just missing a way of dealing with navigation events. His branch already works as a functional blocker, but suffers from the issue of incorrect filtering of requests that occur very early, like the motherjones.com amazonaws example.

So I think any fix needs to go into that branch.

[gorhill]

@AlexVallat Ok, somehow I got into my head that this was the only problem to fix for a Fennec port.

[John-O-Really]

Lovely to see this is being worked on. Thanks for your hard work guys, I'd love to use Ublock on Firefox Android.

[Deathamns]

@chengsun Are your changes ready to be integrated into the main project? Or do you wish to distribute it on your own? Because I looked at it, and if you're okay with it, I could include it with a few changes.

[ghost]

Here are my feedbacks and suggestions. Sorry for my pool English. I have tested chengsun's version. Filter works almost perfectly and the performance is excellent. However, because of the difference between mobile and desktop version. Something should be improved. 1.The dashboard opition should show in the add-ons setting. 2.The switch that choose to enable and disable on a site should be add to the menu of Firefox or address bar. (Just like the ABP on firefox mobile.) 3.The function that used to hide a specific element should be add. This may be difficult to port to android, because it needs to use the touch screen. But it is very useful and convenient.

[chengsun]

@Deathamns: I probably won't have time to work on it further any time soon, so I'd be happy for you to merge this before it bitrots completely. Obviously there are still the issues mentioned above, but we can always build on it later.

[Deathamns]

@chengsun There. I restyled the code, and made some other changes, but I can't test it, so if you'll have the time, take a look if I messed up something.

[TheReverend403]

@Deathamns just tested, it installs but no longer blocks anything.

[polyzen]

Same results as @TheReverend403 (tried XDA and the quick tests)

[AlexVallat]

https://github.com/gorhill/uBlock/blob/master/platform/firefox/vapi-background.js#L369 should be

tabId: getOwnerWindow(win).BrowserApp.getTabForWindow(win).id,

Alternatively, the whole onFennecLocationChange handler can be removed following @gorhill 's implementation https://github.com/gorhill/uBlock/blob/70488274b1d1a1ddfbdafd5c5ad65a2a55871dac/platform/fennec/vapi-background.js#L1102 if we don't care overly about #516 (which is not such a big deal). See also my comments earlier in this issue

[AlexVallat]

Found some other bugs too. Probably easiest if I just point you at a version of vapi-background.js I've fixed up in a clone and you can grab it or diff from there: https://github.com/AlexVallat/uBlock/blob/master/platform/firefox/vapi-background.js

[chengsun]

Cool. One other thing I spotted: https://github.com/AlexVallat/uBlock/blob/master/platform/firefox/vapi-background.js#L436 should be

vAPI.tabs._remove(tab, getTabBrowser(win));

[Deathamns]

@AlexVallat In the above discussion @chengsun mentioned that WebProgressListener could be used in Fennec. If that is true, then it would solve the whole onNavigation issue (because it works exactly like in Firefox, except it needs to be attached to every tab). Someone should test that.

browser.contentTitle is available in Firefox too, however it is empty for tabs that aren't yet loaded until you click on the tab (usually after browser start). tab.label seems to always have something, I assume this is the case with Fennec too. However if tab.label is broken in Fennec, then first you should test if browser.currentURI.asciiSpec is available in Fennec, because then we won't need a condition for the two platforms (since it works in Firefox).

[AlexVallat]

@Deathamns tab.label is not available in Fennec. The only choices for getting the tab title are browser.contentTitle or tab.window.document.title (which I'd say is worse).

browser.currentURI.asciiSpec is fine, but I'm not sure how that helps us?

[Deathamns]

@AlexVallat browser.currentURI is available in Firefox too, so we don't have to use different code for the two platforms.

[AlexVallat]

@Deathamns Unless I'm missing something, we aren't... https://github.com/AlexVallat/uBlock/blob/master/platform/firefox/vapi-background.js#L574

Or was there somewhere else you were thinking of?

[Deathamns]

@AlexVallat Sorry, I'm stupid. I looked at the wrong line.

[AlexVallat]

I've coded up some UI support for Fennec, in my fork. In the Add-on Options there are now two buttons, one to launch the dashboard (what the Options button used to do), and one to launch the request log. This was necessary because Fennec only supports inline options, so it's not possible to just use the options button to go straight to the dashboard.

I couldn't find any way of separating out behaviour for Fennec and desktop firefox, which means you get the same inline options buttons on the desktop, but I don't think this is a big deal.

For the toolbar button, I've added a tools menu item. There is no icon, but it can display the number of blocked elements after the name in parentheses, so that information is not lost. There is no support for popups (and the screen area is probably be too small for them to be useful anyway) so I've had it open a new tab with the popup UI in it when you click the menu item. Obviously not as convenient as being able to see it, and the page it applies to, simultaneously, but it is at least workable.

It has required some plumbing changes to the non-platform specific code too, but nothing that should break anything else.

[gorhill]

It has required some plumbing changes to the non-platform specific code too, but nothing that should break anything else.

Can I have details of where and why?

[AlexVallat]

Can I have details of where and why?

In popup.js and messaging.js, to allow the popup to work as a tab, rather than a popup, I added support for passing in the tabId as a parameter, and for switching back to the tab it applies to when refreshing or launching the picker. I also added the tab title to it's window title, to make it easier to identify which tab it was for.

In popup.html, devtools.html and dashboard.html, to add a meta tag so that mobile browsers display them at a more appropriate viewport (effectively zoom level) by default.

I'm not sure whether it counts or not (or exactly what your policy is on localisable strings), but I added some new strings to en/messages.json too - they are only used by the Firefox specific code though.

[privacy1776]

I see some changes being made, when is it expected to be released?

[polyzen]

@privacy1776, I'm pretty sure you could just install the dev release xpi (though the release notes don't mention Fennec support). Otherwise, you could build the xpi yourself, copy it to your device/s, and open file:///path/to/xpi in Fennec

[privacy1776]

I have installed the dev release, I figured out how to access the settings, but I'm unable to change or edit any filters, I suppose this is in the works as we speak

[TheReverend403]

@privacy1776

chrome://ublock/content/dashboard.html

[ghost]

I would like to see this happen. HTC One was very hard to root, can't use universal adblocker.

[gorhill]

Official support will be in 0.9.1.0.

[my-password-is-password]

Thanks @AlexVallat, @chengsun, @Deathamns, and @gorhill !

:thumbsup:

[xiongchiamiov]

I haven't chimed in on this thread, but I've been anxiously following it, and I'm very :smiley: right now. Thanks everyone!

[ghost]

Simply amazing, gorhill and team. The ublock UI for Firefox on Android is simply phenomenal and the willingness to support more and more platforms is what cements ublock as my favorite adblocker. Keep it up