uBlock Origin breaks custom toolbars in Palemoon

[drivetheory]

uBlock Origin 1.3.4, 1.3.5, 1.3.6, 1.3.7 (dev build)
Palemoon 25.8.1 (x64)

my testing method

1. create new blank pale moon profile
2. create new custom toolbar, put Location component on it.
3. install ublock origin
4. open pale moon via pinned icon on task bar
5. check for custom toolbar with location component visible
6. press CTRL+W to close window
7. repeat step 4-6 until custom location toolbar disappears (takes 3-4 open+close operations)

i also posted this on the Palemoon forum to which the developer did repsond.
http://forum.palemoon.org/viewtopic.php?f=15&t=10352

[gorhill]

Unable to reproduce.

Any error message in the browser console?

Also I can't make sense of Moonchild's comment, uBO does not block chrome elements, it blocks HTTP requests or hide DOM elements on http/https pages only.

[gorhill]

Closed by mistake clicking wrong button.

[drivetheory]

today during my free time off and on I had the chance to test it on a handful of computers I have access to.
3 Win7 (x64) & 1 Win2012R2 (x64).

I could successfully reproduce the problem on 2 of the Win7 machines as well as the Win2012R2 machine. I could not reproduce it on the 3rd Win7 machine.

the curious part is that the machine i could NOT reproduce the problem on is the only machine NOT using an SSD.

will test more when I have time and note the contents of the browser console.

[gorhill]

the curious part is that the machine i could NOT reproduce the problem on is the only machine NOT using an SSD

I have an SSD -- but I am using linux. Again, when the issue occurs: any error message in the browser console?

[gorhill]

Maybe you could give a try with latest dev build, b12. I just added checks for readyState === 'complete' for the chrome window + delayed adding uBO's custom stylesheet at start time. Since I can't reproduce, I can't tell whether this helps, but strictly from a code review standpoint, these minor changes make sense.

[MajorLunaC]

I have the same problem (I think). I use Firefox ESR 38.4.0 on Linux, and when I enabled uBlock, it erased 3 custom toolbars. It wouldn't be such a problem if Firefox hadn't removed the ability to make custom toolbars now (or maybe I'm missing something in the Customize editor?). They reappeared after I disabled uBlock. You might have to try a Firefox version before they added the new weird toolbar Customize editor, as the only custom toolbars seem to be the ones lingering from previous versions. :(

[drivetheory]

my apologies for not mentioning I was using windows in my very first post.

this time around I installed uBlock Origin 1.3.6 in Palemoon 25.8.1 (x64) on Win7 (x64)

just finished up some testing, to get the error console to log to file i installed an addon called Issuereporter.
http://rokdd.de/b/issuereporter

i set the addon up for the constant logging feature and it created a new HTML log file every time the browser was started and stopped.

I collected, numbered and labelled about a dozen relevant log HTML files and uploaded them here:
http://drivetheory.org/ublock

not sure how useful these will be as they seem a bit sparse. will look for a better browser console to log file addon/method/feature/etc, if you know of one please do tell, i couldn't readily find one

EDIT: added more test results

[gorhill]

First thing that should be addressed is that Pale Moon fixes the following issue, which causes an error message in your console, line 200 in chrome://browser/content/padlock.js:

    document.getElementById("urlbar").setAttribute("https_color", colshow);

document.getElementById("urlbar") returns null apparently sometimes at start time, and this causes the above line to throw. "urlbar" suggests this is related to the URL bar.

The fact that you said it does not happen with your version on a computer without an SSD suggests there is a timing issue. Apparently the line above expects the urlbar element to be present when it executes, but it is not in many cases as per your logs -- and it's occurring for both cases where your URL bar shows or not. Other than this, I see nothing peculiar in the logs.

[gorhill]

I fixed a case of win unexpectedly being null at line 682, but it appears this happens when uBO is shutdown, so unlikely to be the cause of your URL bar disappearing -- also considering there was only one such instance of this occurring in all of your logs.

[drivetheory]

added another 1/2 dozen logs where the location bar is missing and ublock is active, all files prefixed with 10. all the new logs look the same aside from timestamps.

that's the thing though, once the bar is gone the bar is gone until I disable ublock and restart the browser, upon restart the location bar comes back, then a few restarts later the location bar disappears again.

will install the latest dev build and test later before bed.

[gorhill]

@MajorLunaC Are you using the legacy toolbar button? See in about:config if extensions.ublock0.forceLegacyToolbarButton is true.

[gorhill]

The two following errors are reported in your logs for every time you say the URL bar is gone:

TypeError: document.getElementById(...) is null    chrome://browser/content/padlock.js    200
TypeError: document.getElementById(...) is null    chrome://browser/content/padlock.js    200

At line 200 of chrome://browser/content/padlock.js, there is a function, usePrefs, which purpose appears to assign a style to the padlock of the URL bar (I guess the one which identifies a HTTPS site). That usePrefs function is called from two places, from another function called onLoad, and another place from within a user pref observer listener.

uBO does modify some user prefs at start, to enforce the disabling of IP address leakage through WebRTC, network prefetchnig etc. So it appears this could trigger the above listener where usePrefs is called.

Moonchild answer's to your report are not encouraging. Somehow he did notice some entries about uBO -- those are probably inconsequential as they occur at shutdown time, but yet failed to note the most interesting ones, the one related to the URL bar. The file chrome://browser/content/padlock.js seems to be specific to Pale Moon, I do not see it for Firefox nor SeaMonkey.

[MajorLunaC]

I'm not sure why, but I can no longer reproduce the lost toolbars issue (1-time or 2-time thing!?). extensions.ublock0.forceLegacyToolbarButton is false, but toggling it and restarting firefox did not change anything.

As a side-note, it does seem Firefox removed the "Add New Toolbar" button, so I used "Classic Theme Restorer" addon to allow more customization, including adding new toolbars. There have been no issues related to uBlock now.

[LimboSlam]

This what MC said:

The padlock scripting has been in place for a very very long time and has not in any way changed recently.

I can't provide more than generic responses since I didn't write the extension now, did I?

EDIT: if uBlock Origin is Jetpack/Add-onSDK built, then the following change may possibly affect the extension (dealing with allowing scripts in panels): https://github.com/MoonchildProductions/Pale-Moon/commit/a436f3de6c9c6bbed9bc6b2ea24ff0e0e1bc3455

This is a security update that is essential to stay in. If for some reason you define panels that don't have an "allow" statement in there or that specify allow: { script: false }, then script execution will be prevented.

Maybe this help @gorhill and @drivetheory?

[gorhill]

if uBlock Origin is Jetpack/Add-onSDK built

No.

I reviewed the toolbar button code again today, line by line, and there is nothing left to explain the custom toolbar disappearing. So I will leave this as is. Personally I am currently assuming there is an issue specific to Pale Moon, and I won't spend any more time on my side about this -- especially given the flippant response from Pale Moon developer.

[LimboSlam]

Hey @gorhill: I was the one who suggested to him that you might be a little irritated with his response, so don't blame MC, blame me for his response back. And ok I was just trying to help you guys communicate better, but alright.

[gorhill]

blame me for his response back

Of course not.

What I was expecting is some technical feedback as to why the urlbar element is not found as per the logs provided -- this is unexpected and cause a throw. So far this is the best lead to try to find out why the toolbar disappears. (If I understand correctly, Firefox/Pale Moon automatically removes empty toolbars -- so if a toolbar has only the url bar element and it is not present, the toolbar will be removed.) Responding with "I didn't write the extension now, did I?" accomplishes absolutely nothing for resolving the issue.

uBlock does not remove anything from toolbars, it just inserts itself onto whatever toolbar the user wants it to be, nothing more, so I fail to see how the Pale Moon element urlbar is unexpectedly absent when the Pale Moon's onLoad listener in Pale Moon's chrome://browser/content/padlock.js script is executed.

On the other hand I don't think there were any tests using this change here, since only @drivetheory can reproduce the issue only him can tell whether this makes a difference or not.

[LimboSlam]

Well I don't much about coding, but I'm pretty sure you are correct that Firefox/Pale Moon automatically removes empty toolbars if a toolbar has only the url bar element and it is not present, the toolbar will be removed. Yeah I also can't reproduce this issue either, so we're just waiting on @drivetheory to confirm anything.

Hey @wolfbeast can gives us some more insight this, what do you say MC?

[drivetheory]

sorry for the late reply, didn't have anytime to myself this weekend...

installed the latest dev build of ublock origin. added 2 more log files as 11 (working) & 12 (toolbar gone).

will try to do some testing on my 2012R2 box tomorrow so that there's another set of results. and, again, if you know of a better logging method i could use please feel free to suggest it.

EDIT: server wasn't in use so i messed around a bit before bed using dev build of ublock origin, posted results in .\2012R2 and if the toolbar was missing the file names got appended with "(missing)"

EDIT 2: I also added the activity indicator to the same toolbar as the URL bar so that there were 2 components, both went missing and the toolbar disappeared.

[gorhill]

I carefully reloaded all the logs, and the first error occurring which is common to all the ones where the toolbar is missing is:

this.docShell is null    chrome://global/content/bindings/browser.xml

Still this does not provide any clue as to what causes this.

So given this, I have to consider this issue unresolvable, unless someone comes with useful information eventually.

Someone mentioned on the Pale Moon forum to get a similar issue, and found that installing Pale Moon 25.8.0 the issue disappeared, might be worth giving this a try.

Sorry that I could not be of any help.

[gorhill]

Finally, out of nowhere I reproduce the issue, my URL bar is gone, with similar error messages in the browser console:

[10:19:18.353] TypeError: document.getElementById(...) is null @ chrome://browser/content/padlock.js:200
[10:19:18.588] ub is null @ chrome://browser/content/padlock.js:77
[10:19:21.828] ub is null @ chrome://browser/content/padlock.js:77

[gorhill]

I've narrowed the issue to one single instruction: merely looking up win.gBrowser at start up time will cause the issue. Not looking up win.gBrowser at start up time will not cause the issue. This is true regardless of what uBO does.

The mere lookup of the gBrowser property on the window at launch time is what causes the issue:

function getTabBrowser(win) {
    void win.gBrowser;
    return null;
}

= Custom bar does not appear.

While:

function getTabBrowser(win) {
    return null;
}

= Custom bar does appear.

The issue started to be completely reproducible on my side after I decided to re-factor a little section of code which is not critical to be executed at launchtime, in order to defer the execution and hence remove a bit of overhead from uBO launch time.

So something is occurring in the Pale Moon gBrowser getter which causes the issue.

[wolfbeast]

I'll have a look at the code involved, but the getter for gBrowser is stable code that has not been touched in... forever. It makes very little sense, but if it's an issue then it may be something caused by timing changes in the browser (potentially because of removing Mozilla telemetry or the likes speeding things up). Probably an inherited issue with touchy code that never hit a snag at Mozilla.

Thanks for narrowing this down to the getter; without the specifics in this issue this would never have been pinpointed sufficiently to have a clear understanding where to look.

As for the padlock code: that makes perfect sense to throw; if the element isn't there because the toolbar isn't there, then the padlock code will throw when trying to set attributes. Doesn't mean the padlock code is at fault, just means it can't access what it expects to always be there.

EDIT: I had a look at the browser.js init code and win.gBrowser's getter (and setter) is initialized first thing, right at the top of the browser code.
Firefox still does the exact same thing too (I checked FF42 source code since I have it handy) with the exception that it defines a whole bunch of other things as lazy getters beforehand, although none of those would be related to extensions, AFAICT. The add-on manager is still initialized at the same time as it is in our code.
With @MajorLunaC reporting the same in FF38 ESR, my guess is that this is still a potential issue in Firefox as well when things are initialized with very little latency.

[wolfbeast]

Oh silly me, I forgot that it's not an issue for Firefox any longer because custom toolbars don't exist anymore. As the credo goes: "Removed code is debugged code".

I've been able to reproduce this issue with 1.3.6 also on the latest Goanna trunk builds of Pale Moon, so it's not a fluke of a single build. Not that I have been able to find a way around it, yet, or even an idea how to avoid this issue. I hope you don't mind keeping this hack in place for a while.

[gorhill]

Doesn't mean the padlock code is at fault, just means it can't access what it expects to always be there.

Isn't a fault to try to access the URL bar without first checking if it's there? The unhandled exception in there cause the javascript execution to abort -- leaving exceptions unhandled does not sound like a good approach to me. For example, should an exception be thrown at line 200, this means the preferences observer won't be installed, I am pretty sure this is not the intended behavior.

If you look in chrome://browser/content/browser.js, pretty much everywhere there is a test of gUrlbar to see if its valid before it is used, implying the URL bar element not being present is something which can happen under normal circumstances. In chrome://browser/content/padlock.js, I do not see such tests at line 73 (where it throws on my side), or at line 200 (where it throws for OP and me). Currently what has been observed so far here is that where an exception is thrown at these lines, the URL bar-in-a-custom-toolbar fails to appear.

[wolfbeast]

You're absolutely correct that the padlock code should check if the element is present before trying to change it.

But, it's the other way around. It fails to check (which I've since corrected, actually, in a commit today) if the URL bar exists before using it, so the overlaid functions to set properties fail (which won't influence the browser's core scripting otherwise). That doesn't influence the toolbar being present or not, it just means that it is a symptom of the toolbar not being there.

I've tested this as well by putting it in try/catch blocks in a built version, which prevents the errors, but makes no difference as far as uBlock Origin making the custom toolbar disappear as well as causing issues with lightweight themes when there's a low latency.

I have to ask though, at what point in the browser's startup do you inject your code? Are you directly tying it into the appshell or something? (If that's the case, then please understand that things simply won't be guaranteed to be ready, including gBrowser and other globals, and there's very little I can do about changing that without completely rewriting the structure of the browser xul/core js, which I'm not going to do).

[gorhill]

That doesn't influence the toolbar being present or not, it just means that it is a symptom of the toolbar not being there.

Ok I see. So things are more complicated internally and I get that starting to touch all this is to be avoided. uBO is a bootstrapped extension, so its code starts to execute whenever the browser calls the startup() function in its bootstrap.js file.

The current workaround in uBO is trivial enough, I don't mind keeping it around -- it comes with a large notice explaining the purpose -- and it might end up also addressing whatever other difficult to reproduce issues which may have been reported for other flavor of FF browsers.

[wolfbeast]

I've researched this some more, and as far as I can gather, it may be a result of Mozilla's obsession with startup telemetry and shaving off as many ms as they can, and doing as much async and lazily as possible. From what I have been able to gather thus far from the behavior seen, it's a problem that the getter gets called by your startup script while it is already in the process of being gotten and filled, so it'd be a kind of race condition, or rather overlapping getter calls, clearing and re-getting the object from the call from uBO which breaks previously initialized code by removing the global object to repopulate it (as a "smart" getter does). Highly timing sensitive, and predictably hard to reproduce unless the conditions are just right.

Also seems we're not the only ones having issues with this kind of thing. While investigating, I also ran across a SeaMonkey bug on Bugzilla dealing with something very, very similar.

I do think your current workaround would avoid similar issues in other Mozilla-based browsers. It's likely a good idea to keep it in place to avoid these issues.

[wolfbeast]

FYI, something else to keep in mind:

https://bugzilla.mozilla.org/show_bug.cgi?id=1161538

--- Comment 6 from Philip Chee 2015-12-10 14:25:27 PST ---
Well uBlock Origin should wait for onload since before onload there won't be
any tabs with content so trying to grab gBrowser that early doesn't help.

[gorhill]

Well uBlock Origin should wait for onload

Only problem is that onload won't be triggered if uBO is enabled after the browser is up and running. The current solution is best, it will work in both scenarios where uBO is launched at browser start up time or after the browser has been started.

[wolfbeast]

Awesome then! I'm sure everyone running into this will be thankful :)