philikon / BarTab

Firefox add-on: Drink now, pay later: put your tabs on your bar tab!
https://addons.mozilla.org/firefox/addon/67651/
64 stars 30 forks source link

Working & Tested Firefox 4 support #119

Open claudenobs opened 13 years ago

claudenobs commented 13 years ago

At least pull it and make a beta for those craving firefox 4 support :)

These changes have now been throughly tested by me using two profiles (one approx. 150 tabs, 10 tabgroups, ~20extensions, the other fifty tabs one tabgroup, no other extensions). Has some quirks when using gotoLastLoadedTab and this tab is in another tabgroup: then the unloaded tab may switch group.

philikon commented 13 years ago

I can't get this to work properly :(. With BarTab's standard settings, if I click on a tab that hasn't been loaded yet, it doesn't load. Am I missing something?

claudenobs commented 13 years ago

Interesting. I'm running the official 4.0 Firefox (32&64bit) on OSX 10.6.7. I just tested again with a new profile and just bartab with its default settings and confirmed that it works here. Same applies for a second mac (latest osx as well). Just checked in a Windows XP VM that this is a platform specific behaviour...

"if I click on a tab that hasn't been loaded yet, it doesn't load" - thats what happened when i use the unpatched bartab with ffx4, which is why i started hacking it in the first place... :)

xabolcs commented 13 years ago

Hi there,

I am just a watcher of this repo, but want comment to this pull request!

All of my test was on Win64: FF4.0.1 (empty profile) and Nightly 6.0a1 (2011-04-26) (used profile). I tested just the "BarTabbing the tabs on startup" ;) function.

Test Case 01: empty run 1: get blunet/BarTab@1565e6a0e214cdefeffbef5c244a992107ed3d73 2: install it FF4 says: "browser.webNavigation.setCurrentURI is not a function in resource://gre/components/nsSessionStore.js at line 2828" Nightly says it too.

Test Case 02: pseudo setCurrentURI 1: define useless setCurrentURI in BarTabWebNavigation() like mine: https://gist.github.com/943267 2: test it FF4 gets working: clicking on an unloaded tab makes it loaded :) Nightly differs: the about:xxxxx tabs gets loading, but the others don't load themselves. (ErrorConsole says: BT.setCurrentURIed: about:blank all of unloaded-restored tabs)

Test Case 03: passthrough setCurrentURI 1: "rewrite" the previous setCurrentURI() like that: https://gist.github.com/943273 2: test it :) FF4 still works like a charm Nightly works as in Test Case 02: the about:xxxxx gets loaded, others don't.

I hope this comment helps you guys! :)

claudenobs commented 13 years ago

interesting. i already discarded that as being irrelevant, cause i got this on OSX as well, but fixing it up fixed some weird tab duplication when unloading tabs yet affects none of the tabs not restoring... but now it may need more investigation... thanks. if you like, try applying this patch : https://bugzilla.mozilla.org/show_bug.cgi?id=647495 to your omni.jar. (You need to unzip your omni.jar (resides somewhere in Programs/Firefox/... at least on OSX), apply the patch, rezip and rename to omni.jar). hopefully i'll have time to look into it this weekend...

xabolcs commented 13 years ago

I "patched" (=modify webnav to docshell) the omni.jar in my FF4.0.1 After patch it doesn't call my pseudo setCurrentURI(). :) And of course your fork gets working!

With Nightly I have no success with it. I do not know what it does with the omni.jar. I patched it, and restarted (browser, os), but it reclaims for bad webnav, but clicking into the ErrorConsole it shows the pached, docshell version. So with Nightly I was not able to test your patch. :(

Anyway, your first aim is FF4, isn't it? :)

claudenobs commented 13 years ago

I "patched" (=modify webnav to docshell) the omni.jar in my FF4.0.1 [...] your fork gets working!

thx. Great. or not. as i see it, the current implementation of bartab is definitely nuked by a FF4 bug. my primary aim was to get it working on FF4, which it does with the omni-hack. for now. it seems like a proper solution will take more time

xabolcs commented 13 years ago

hmmm.

imho the omni.jar-hack isn't required (but recommend) for FF4: if you implement the "passthrou" method as I said then it will work (for FF4). :)

of course, the compatibility with Aurora and Nightly is more complicated, as you mentioned. see Giorgio's comment about webNavigation in a topic of an old bug in LoadTabsProgressively. he said: The "original" browser.webNavigation implements nsIDocShell. so the next question is: all the docShell property, function, etc. is implemented in BarTab's webNavigation? the test for this (proposed by Giorgio): alert(top.opener.gBrowser.selectedBrowser.webNavigation instanceof Components.interfaces.nsIDocShell) and testing with BarTab it says true which is a good point. :)

claudenobs commented 13 years ago

he said: The "original" browser.webNavigation implements nsIDocShell.

interesting. after a little more documentation reading : BarTab replaces each tabs browser.webNavigation with BarTabWebNavigation, but BarTabWebNavigation only implements nsIWebNavigation but not nsIDocShell since the browser.webNavigation doc say it only implements nsIWebNavigation.

is all the docShell property, function, etc. is implemented in BarTab's webNavigation? [...] testing with BarTab it says true

unfortunately the test lies. instanceof will call BarTabWebNavigation.QueryInterface() wich just passes the call through to _original (browser.webNavigation). However this means that browser.webNavigation says it implements nsIDocShell, so either browser.webNaviagtion lies or actually implements docShell and maybe even is the same object as browser.docShell.

imho the omni.jar-hack isn't required (but recommend) for FF4: if you implement the "passthrou" method as I said then it will work (for FF4). :)

sorry just overlooked this... this might be the correct approach, and certainly avoids the bug. (continuing from above:) Im beginning to think that it is the latter since the bug is caused by nsSesstionStore.js (omni.jar) calling browser.webNavigation.setCurrentURI() which is part of nsIDocShell. Since BarTabWebNavigation does not implement and/or pass through nsIDocShell the error arises. Likely even browser.webNavigation == browser.docShell because browser.webNavigation.setCurrentURI() works like browser.docShell.setCurrentURI()...

With Nightly I have no success with it. I do not know what it does with the omni.jar. I patched it, and restarted (browser, os), but it reclaims for bad webnav, but clicking into the ErrorConsole it shows the pached, docshell version.

Maybe firefox nightly just more agressively caches omni.jar... have you tried starting firefox with "--purgecache" option? And is there an error when using your passthrough method in nightly/aurora?

claudenobs commented 13 years ago

One other thing about Aurora/Nightly : if it doesn't work with the passthrough of setCurrent() then i think at some point there has to be a patch for nsSessionStore.js because it calls browser.webNavigation.setCurrentURI()...

xabolcs commented 13 years ago

ehh! my newly developed omni.jar-hack v2 super-duper method brings your pull request to life under nightly! :)

step1: get blunet/BarTab@1565e6a step2: "patch" omni.jar/nsSessionStore.js to docShell step3: delete omni.jar/jsloader/resource/gre/components/nsSessionStore.js.bin from archive step4: (re-) start Nightly with -purgecaches and BarTab works as it should

by the way sadly i can't reproduce my test case 01 & 02 with actual Nightly (Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a1) Gecko/20110502 Firefox/6.0a1 ID:20110502030625) and with actual Aurora (Mozilla/5.0 (Windows NT 6.1; WOW64; rv:5.0a2) Gecko/20110502 Firefox/5.0a2 ID:20110502042005): they work as FF4. hmmm. i tested with proxy file ... trying out with makexpi.sh-ed xpi. ... there is no difference.

@philikon, @blunet could you try out the passthrou workaround and the omni.jar-hack v2 method against actual (and/or some older) Nightly and Aurora? now i am not sure about myself.

thanks!

checking blunet/BarTab@1565e6a + passthrough against FF4, 5, 6: OK: Mozilla/5.0 (Windows NT 5.1; rv:6.0a1) Gecko/20110428 Firefox/6.0a1 ID:20110428030557 OK: Mozilla/5.0 (Windows NT 5.1; rv:6.0a1) Gecko/20110503 Firefox/6.0a1 ID:20110503030636

OK: Mozilla/5.0 (X11; Linux i686; rv:2.0.1) Gecko/20100101 Firefox/4.0.1 ID:20110413222027 OK: Mozilla/5.0 (X11; Linux i686; rv:6.0a1) Gecko/20110427 Firefox/6.0a1 ID:20110427030633 OK: Mozilla/5.0 (X11; Linux i686; rv:6.0a1) Gecko/20110503 Firefox/6.0a1 ID:20110503030636

OK: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a1) Gecko/20110502 Firefox/6.0a1 ID:20110502030625 OK: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a1) Gecko/20110503 Firefox/6.0a1 ID:20110503030636

xabolcs commented 13 years ago

After the Nighlty (6.0a1) -> Aurora (6.0a2) merge things gets bugging. :(

For example with Mozilla/5.0 (Windows NT 5.1; rv:6.0a2) Gecko/20110602 Firefox/6.0a2 ID:20110602042006 I getting Error: browser.webNavigation._pauseLoadURI is not a function Source File: resource://bartab/prototypes.js Line: 693 in some circumstances: 1.: create some tab and some group with panorama 2.: restart browser to get the tabs in unloaded state 3.: changing to a tab in another group with panorama 4.: click on an another unloaded tab using the tabbar (ergo: inside the tab group) 5.: error console logs the error, and the throbber spins counter clockwise 6.: after ESC-ing, or stopping the loading thobber spins clockwise 7.: to load the page ENTER hitting is needed into the URL bar

I doesn't have any clues about this. Why does not find the _pauseLoadURI() in the wrapped webNavigation object. :(

In FF5 beta it works as before.

I am going to find the first version of Aurora and Nighlty that causing this.

qwerty017 commented 13 years ago

It may help and it may not but just wanted to add that you may want to look at Load Tabs Progressively to see how they did it as it does work on the nightly to stop tabs from loading until clicked on if you set your settings to only have 1 tab loaded. This also works when opening new tabs and keeps them from loading until clicked on. Just an FYI in case it helps.

xabolcs commented 13 years ago

I digged myself into it.

Extending BarTabWebProgressListener.onStateChange() with this gist will do the trick.

xabolcs commented 13 years ago

Error: browser.webNavigation._pauseLoadURI is not a function Source File: resource://bartab/prototypes.js Line: 693

IMO the landing of Panorama Groups On Demand caused that.

xabolcs commented 13 years ago

Opening a tab from History / Recently Closed Tabs menu or with accel + shift + T shortcut causes another _pauseLoadURI() error: Error: browser.webNavigation._pauseLoadURI is not a function

Magyver commented 13 years ago

@blunet, Re: "if I click on a tab that hasn't been loaded yet, it doesn't load" ..... It's not supposed to load automatically, remember it is disconnected from the CPU.

Merely hit the refresh button when FF loads, or you choose a different tab. Each tab will load in mere seconds because Bar Tab shuts down the FF memory usage so well.

BTW, are all you guys using the newest version? I think not. The new version works across all the FF versions, 3.6 thru 6.0, it works GREAT in FF 5.0, loaded it today.

It's Version 2.1b2. It downloaded in 60 secs, restarted & works great!

Tabs close automatically 60 secs after leaving them. Unused tabs are greyed out. The default parameters are fine.

Load it from this page: https://addons.mozilla.org/en-US/firefox/addon/bartab/versions/

Bless you Philippe ! ! Thanks so very much.

Magyver commented 13 years ago

Remember that it's a small price to pay to hit refresh on your tabs as you select them. FF 4.0 & 5.0 are horrendous memory gobblers.

I cannot function as a sports writer without Bar Tab. I like to keep from 2 to 3 hundred tabs up with all my sports news sources and websites I write for.

Thank you again Philippe.

fhoech commented 12 years ago

The diff attached to this pull request together with the following changes does the trick for me in FF 5.0 (Windows):

  1. Added passthrough setCurrentURI like above
  2. Replaced browser.webNavigation._pauseLoadURI with browser.webNavigation.loadURI
Doobla commented 12 years ago

I don't fully understand how to add the passthrough setCurrentURI. For instance, step2 instructions mean nothing to me. fhoech, would you be so kind as to fork this and apply the changes that allow it to work?

fhoech commented 12 years ago

Done: https://github.com/fhoech/BarTab

Doobla commented 12 years ago

fhoech, thanks. I don't think it works quite right yet. I am using FF6 beta on osx. As a test I used the session manager extension to load a session with 900+ tabs.

Under FF3 and bartab 2 the browser only uses a couple of hundred MB and the session is loaded quite quickly, evidence that the tabs are truly not loaded upon startup.

Under FF6 and this release I can see each of the tabs loading upon browser startup (evidenced by the fact that I am prompted for invalid SSL certificates to be accepted, etc.) and then after the session is fully loaded I see the tabs all grey out as if unloaded and when I click on a tab it loads it.

Also of note is that at this time the browser uses close to a GB of RAM and 1.2 GB of Virtual Memory. As I click on tabs it only increases from there.

Lastly, for the discerning eye it is obvious when a page is being refreshed vs when it is being loaded from a cache. Some sites will require you to login again if you refresh but simply loading an unloaded tab will not cause this normally, for example. It appears that the tabs are not actually being loaded in the same way that bartab traditionally has loaded tabs which changes the entire experience (and usefulness) of the plugin.

Some things to consider... Thanks for your help.

fhoech commented 12 years ago

As I wrote, only tested with FF 5 under Windows. Here my mem usage is about 360 MB with two active tabs (128 not loaded tabs in the background), which is about what I'd expect when comparing to FF 3.6. edit I noticed I must've set browser.sessionstore.max_concurrent_tabs to 0 in about:config at some point. Maybe thats why it seemed to be working? Just tested it with set to 3 (default). Still works, so I'm guessing this is either a Mac-specific problem or it just doesn't work with the latest nightlies (and presumably FF 6 when it is released).

Doobla commented 12 years ago

Understood. I'm just providing more information. There is a possibility that my test session is partially corrupt from previous tests as well.

Doobla commented 12 years ago

FYI, same results under FF5. I'm going to try a brand new profile without the session manager plugin since that could be affecting the initial loading of the tabs. Also, for documentation sake, automatic unloading of tabs after x time works under FF5 but not FF6 beta.

qwerty017 commented 12 years ago

Tried changing zip to xpi and couldn't install. Any chance of telling me how to make the xpi so I can test on 8.0?

Doobla commented 12 years ago

There is a makexpi.sh script that you run that builds the xpi for you.

qwerty017 commented 12 years ago

Perfect. Thanks. Works in 8.0 except for one thing. If a tab is set to load at startup everything works exactly for that tab. If it is not then the back button stays inactive unless you right click and go back a page at which point that tab will allow you to use the back and forward buttons as usual. Hitting Backspace also does not allow you to go back unless you do the right click thing as above at which point it will work normally nd allow you to back with the backspace key. This also affects tabs tat are set to not load in background. This was tested on the latest 8.0 nightly with a clean profile on latest Ubuntu alpha.

On Fri, Jul 22, 2011 at 10:50 AM, Doobla < reply@reply.github.com>wrote:

There is a makexpi.sh script that you run that builds the xpi for you.

Reply to this email directly or view it on GitHub: https://github.com/philikon/BarTab/pull/119#issuecomment-1633733

Life is a tragedy for those who feel, and a comedy for those who think.

robsonsobral commented 12 years ago

I tried to check for the nearest loaded tab at the same tabgroup, but I don't have the expertise. Some idea how to do this? linkedpanel == selectedPanel?

robsonsobral commented 12 years ago

Done!

r-a-y commented 12 years ago

I can confirm that @fhoech 's build works for me in FF 7 (when the version number is bumped up): https://github.com/philikon/BarTab/pull/119#issuecomment-1630982

Barleyman commented 12 years ago

Guys, could you please place a prebuilt xpi to github so those without linux can use it too? What about adding the fork to the actual addons page?

Barleyman commented 12 years ago

Well, I finally just wrote windows batch file based on the makexpi.sh file. Builds this archive just fine if you have 7zip installed: https://github.com/fhoech/BarTab

del /F /S /Q build release.xpi
md build

copy chrome.manifest.rel build\chrome.manifest

copy install.rdf build
copy icon.png build
xcopy /s defaults build\defaults\
xcopy /s modules build\modules\
"c:\program files\7-zip\7z" a -r build\bartab.jar content locale
"c:\program files (x86)\7-zip\7z" a -r build\bartab.jar content locale

cd build
"c:\program files\7-zip\7z" a -r ..\release.xpi *
"c:\program files (x86)\7-zip\7z" a -r ..\release.xpi *

cd ..
DAOWAce commented 12 years ago

A long shot, but here goes..

Any chance someone can check the source of UnloadTab (only known non-fishy location: http://www.softpedia.com/get/Internet/Internet-Applications-Addons/Mozilla-Extensions/UnloadTab.shtml) and compare the 'unload tab' -> 'switch to active tab' behavior and update this BarTab version to use it? https://github.com/fhoech/BarTab

I'm struggling with my update to Firefox 12 from the amazing but heavily outdated 3.6 and BarTab was one of the addons I used religiously.

Finding the load without refresh fix was great, but there's still this one bad problem.

When unloading a tab, regardless of setting the option to do nothing or switch to the closest active tab, it just loads the tab directly to the right of the tab you unloaded. Obviously, this makes unloading tabs quite useless.

The UnloadTab addon correctly switches to an active tab when a tab is unloaded, but there's a critical design decision flaw with it; when you unload the second tab it just switches back to the first tab you unloaded and loads it again. It repeats this behavior until you click on a different tab and then it uses that tab for the second tab.

So, if there's some knowledgeable person who could combine the switching feature of UnloadTab with that BarTab source, we can finally have BarTab working properly again (to my knowledge at any rate).