Whitelist Options GUI suggestion?

[barbaz]

With all these reports of long whitelists causing the browser to hang when opening Options, and then seeing this post... it occurred to me, would the hanging issue be mitigated at all by using a custom nsITreeView to display the whitelist instead of a listbox? Because an nsITreeView only tries to display what's visible (rather than trying to display all data), and Adblock Plus subscriptions, which could easily be tens of thousands of entries, are viewed by custom nsITreeView and don't exhibit a similar issue...

[Giorgio Maone]

It's been in my TODO list for a long time now, but never got the time and patience for it (nsITreeView's API is quite hairy).
Feel free to submit a patch, or I'm gonna try harder...

[barbaz]

Think I'll give it a try a bit later and let you know how it goes. I'll see if the nsITreeView implementations & utils I have in my own extensions can be adapted to fit NS.

[barbaz]

Looks doable so far but it will take some time, a few days at least. I'm working with NoScript 2.6.8.42rc3 as the base.
Also, given what happened last time I gave you a patch for the Options dialog, I'm not sure if I will be able to debug it for Fx 3.6.28...

[barbaz]

OK, got the basic implementation working, just have to work out a few kinks and make sure it doesn't behave differently from before.

Why do you say nsITreeView is "hairy"?

[Giorgio Maone]

Why do you say nsITreeView is "hairy"?

I just mean that albeit elegant, MVC and all, for the simplest use cases (like this) it usually forces you to write what may feel like an unnecessary amount of code.

[barbaz]

I didn't feel that there was a huge amount of needless code involved...

Anyway, patch sent by PM

[Giorgio Maone]

Thank you so much

[barbaz]

You're welcome.

Does that actually reduce or fix options hanging with really large whitelist in Gecko >= 31?

[barbaz]

Done in 2.6.9rc1, thanks. Hope it works better now

[barbaz]

Oh, BTW, FYI, looks like a few spurious files made it in 2.6.9rc1 as a result of applying this patch:

Code: Select all

Only in ./noscript-2.6.9rc1.xpi_FILES/chrome/noscript/content/noscript: noscriptOptions.js.orig
Only in ./noscript-2.6.9rc1.xpi_FILES/chrome/noscript/content/noscript: noscriptOptions.js.rej
Only in ./noscript-2.6.9rc1.xpi_FILES/chrome/noscript/content/noscript: noscriptOptions.xul.orig
Only in ./noscript-2.6.9rc1.xpi_FILES/chrome/noscript/content/noscript: noscriptOptions.xul.rej

I don't see anything in the diff that references those...

[Giorgio Maone]

Oh, BTW, FYI, looks like a few spurious files made it in 2.6.9rc1 as a result of applying this patch:

Code: Select all

Only in ./noscript-2.6.9rc1.xpi_FILES/chrome/noscript/content/noscript: noscriptOptions.js.orig
Only in ./noscript-2.6.9rc1.xpi_FILES/chrome/noscript/content/noscript: noscriptOptions.js.rej
Only in ./noscript-2.6.9rc1.xpi_FILES/chrome/noscript/content/noscript: noscriptOptions.xul.orig
Only in ./noscript-2.6.9rc1.xpi_FILES/chrome/noscript/content/noscript: noscriptOptions.xul.rej

I don't see anything in the diff that references those...

I had to apply parts of the patch manually, and forgot to remove those files which resulted from automatic patching failure.
Removed now, thanks.