piroor / treestyletab

Tree Style Tab, Show tabs like a tree.
http://piro.sakura.ne.jp/xul/treestyletab/
Other
3.5k stars 278 forks source link

Star UI arrow panel(Edit Bookmarks) does not popup due to landing Bug 951651 #887

Closed alice0775 closed 9 years ago

alice0775 commented 9 years ago

Just for your record.

Tree Style Tab0.15.2015051100a192056 does not work nicely on Aurora40.0a2 and Nightly41.0a1.

After landing Bug 951651, Star UI arrow panel(Edit Bookmarks) does not popup.

tbs commented 9 years ago

When this bug will be fixed?

almet commented 9 years ago

I confirm this doesn't work on nightly as of now.

wenbinleo commented 9 years ago

This bug affects Firefox 40 beta now.

jmozmoz commented 9 years ago

I can also confirm this problem with Firefox 40 beta

alice0775 commented 9 years ago

Firefox 40 has been released. And TST died. Thanks

Miccovin commented 9 years ago

Hi, is it possible to fix this horrible Bug??? I cannot use STRG+D or the Bookmark-Star. This is so essential, that i have to remove Tree Style Tab if this is not working. Hope you will find a solution. :/

My system: xubuntu 14.04, Firefox 40.0, TST 0.15.2015030601.1-signed

mindrones commented 9 years ago

Confiming this bug: appeared after upgrading to Firefox 40.0.

When pressing the star icon, I get this in the Browser Console:

A promise chain failed to handle a rejection. Did you forget to '.catch', or did you forget to 'return'?
See https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/Promise

Date: Thu Aug 13 2015 11:50:56 GMT+0200 (CEST)
Full Message: ReferenceError: aTask is not defined
Full Stack: StarUI._doShowEditBookmarkPanel@chrome://treestyletab/content/bookmarksOverlayEditable.js line 51 > eval:4:9
StarUI.showEditBookmarkPopup<@chrome://browser/content/browser.js:5111:7
TaskImpl_run@resource://gre/modules/Task.jsm:314:40
TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:393:7
TaskImpl_run@resource://gre/modules/Task.jsm:322:13
TaskImpl@resource://gre/modules/Task.jsm:275:3
createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:249:14
PlacesCommandHook.bookmarkPage<@chrome://browser/content/browser.js:5338:1
TaskImpl_run@resource://gre/modules/Task.jsm:314:40
TaskImpl@resource://gre/modules/Task.jsm:275:3
createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:249:14
PCH_bookmarkCurrentPage@chrome://browser/content/browser.js:5431:5
BUI_onCommand@chrome://browser/content/browser.js:6647:7
oncommand@chrome://browser/content/browser.xul:1:1
tbs commented 9 years ago

Sad how this addon is each day more dead, really wish Mozilla had vertical tabs built in to Firefox.

TteokbokkiNari commented 9 years ago

Maybe we can try talking to someone from Mozilla about it, unless someone already did that and got bad results?

mindrones commented 9 years ago

@tbs @Daniellynet I don't think this kind of comments really help motivating developers, no? It's free and open source software after all :)

TteokbokkiNari commented 9 years ago

@mindrones Oh I know, sorry! I just get the feeling that @piroor is busy with other projects right now, and mostly keeps this project on his back burner. :s But this is open source, true. Could have other people trying to fix this, as they have with other issues we've encountered before. I'd help, but my JS knowledge (or coding in general) is practically non-existent... Maybe one day I'll be good enough to contribute to TST.

taoeffect commented 9 years ago

Yes, I also wish Mozilla would pick up this extension and integrate it into FF.

This bug is super annoying. Bookmarking by clicking the star does add the bookmark to the unfiled bookmarks, so you have to now manually open that up, scroll to the bottom of the unfiled bookmarks, and then add keywords there.

eToThePiIPower commented 9 years ago

@mindrones's error log seems to be the key.

I tried a quick fix by deleting lines 50-56 in content/treestyletab/bookmarksOverlayEditable.js, and it seems to quash this bug. I want to dive in a bit deeper to see if there are any side effects before sending in a pull request, since I don't know this code base well (at all, really). There's probably a better way to do it.

taoeffect commented 9 years ago

@eToThePiIPower Very cool, thanks so much for looking into this! crosses fingers (<= BTW, I bet no one can guess how I did the markdown for that. ^_^)

TteokbokkiNari commented 9 years ago

@eToThePiIPower so I decided to test it out by deleting lines 50-56 in the same file, and sure it works now, but only some of it seems to be working? After I bookmark it and press the star again, it seems to be locked in place like so after the pop-up menu comes up: example. My CTRL + W function to close down tabs is broken too after this step.

Firefox version: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:43.0) Gecko/20100101 Firefox/43.0 ID:20150816091433

hofst commented 9 years ago

@eToThePiIPower Can confirm that bookmark window is working when deleting the lines, but ctrl-w is broken in firefox stable 40.0.2. If you had the time to look into this I would be very grateful

The error that might be related to this: "TypeError: namedNodes.counter is null browser.js:1211:4"

Miccovin commented 9 years ago

Maybe someone of you guys know the extension named "userchromeJS". For this there exists a script named addbookmarkinlastusedfolder.js. The Script caused the same Star-/Ctrl+D-bug after updating to Firefox 40. Someone found a solution to fix this in the script - look here (in german): http://www.camp-firefox.de/forum/viewtopic.php?p=975388#p975388 You have to look at the following posts - there is the explanation. Maybe someone helps this to fix the buggy TST finally completely.

eToThePiIPower commented 9 years ago

Thanks for the reports on CTRL-w failing, I noticed that myself soon after I wrote the post.

I tried wrapping the problem lines (51-56) in a try-catch block and that seems to be working better. It seems that the eval expression fails the first time the bookmark button is pressed, but if it isn't run on subsequent times the problem with CTRL-W (and showing "remove bookmark") occur.

I'll be sending up a PR tomorrow hopefully; I want to keep it running locally for the moment just in case there are other problems.

hofst commented 9 years ago

Your PR seems to be working for me :) thanks a lot!

@piroor tested system: windows 10, firefox 40.0.2 stable

piroor commented 9 years ago

Those patching around line 50 can be rewritten with replacing of functions like 80938e8, because they eval() just insert simple method call at the head of each function, However there still other errors...

piroor commented 9 years ago

For developers I describe the background of the commit 4abc4413e98ee99bc1008c29df6414636c990811. I hope that this helps you to resolve bugs like this in future versions.

As the comment of 80938e8 TST fails to patch to gEditItemOverlay.initPanel. Steps to find the change of Firefox which produces this error:

  1. Find the definition of the target method: http://mxr.mozilla.org/mozilla-central/search?string=initPanel
  2. See the code: http://mxr.mozilla.org/mozilla-central/source/browser/components/places/content/editBookmarkOverlay.js#165
  3. See change logs of the file from the "Hg logs" link: http://hg.mozilla.org/mozilla-central/filelog/d5cf4e7900df/browser/components/places/content/editBookmarkOverlay.js
  4. Open "diff" links in tabs and find the commit which removes the line: if (this._itemType == Ci.nsINavBookmarksService.TYPE_BOOKMARK) { I've found it: http://hg.mozilla.org/mozilla-central/rev/d84b62b367b4
  5. Compare the current definition of gEditItemOverlay.initPanel and the removed codes to find where is the correct location to insert our codes.

The old patching aims to execute inserted codes only for simple bookmarks, so, the variable isBookmark seems good information to do it. In other works, we can put our code anywhere after the variable is defined. Then I decided to put my codes before the line: let showOrCollapse =, like:

TreeStyleTabUtils.doPatching(gEditItemOverlay.initPanel, 'gEditItemOverlay.initPanel', function(aName, aSource) {
    return eval(aName+' = '+aSource.replace(
        'let showOrCollapse =',
        'TreeStyleTabBookmarksServiceEditable.initParentMenuList(); $&'
    ));
}, 'TreeStyleTab');

Then separate codes for each Firefox version (See also https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIVersionComparator ) like:

if (Services.vc.compare(Services.appinfo.platformVersion, '40') >= 0) {
    // for Firefox 40 and later, after Bug 951651
    TreeStyleTabUtils.doPatching(gEditItemOverlay.initPanel, 'gEditItemOverlay.initPanel', function(aName, aSource) {
        return eval(aName+' = '+aSource.replace(
            'let showOrCollapse =',
            'TreeStyleTabBookmarksServiceEditable.initParentMenuList(); $&'
        ));
    }, 'TreeStyleTab');
}
else {
    // for Firefox 39 and olders
    TreeStyleTabUtils.doPatching(gEditItemOverlay.initPanel, 'gEditItemOverlay.initPanel', function(aName, aSource) {
        return eval(aName+' = '+aSource.replace(
            'if (this._itemType == Ci.nsINavBookmarksService.TYPE_BOOKMARK) {',
            '$& TreeStyleTabBookmarksServiceEditable.initParentMenuList();'
        ));
    }, 'TreeStyleTab');
}
piroor commented 9 years ago

The commit dd2f17dd5359d6c92ad1e4c8006ca9c5b833b0d4 fixes the error: treestyletab: doPatching: gEditItemOverlay._showHideRows is missing! utils.js:319:0

The commit http://hg.mozilla.org/mozilla-central/rev/d84b62b367b4 removes gEditItemOverlay._showHideRows also. We have to insert our code to new location. Because the new version gEditItemOverlay.initPanel seems to contain the task of old gEditItemOverlay._showHideRows, I decided to insert my code to gEditItemOverlay.initPanel again, at its tail.

piroor commented 9 years ago

My private server for a service hook is still down, so the "nightly build" never been updated automatically. Sorry.

piroor commented 9 years ago

One more commit 5380304aa8688e7d725a206e70188afd1a526e19. I realized that there is no reason to initialize my custom UI when it is hidden, so it becomes to be executed only when the UI is visible.

TteokbokkiNari commented 9 years ago

Thank you so much for the fix, @piroor ! Seems to work without issues on my end!

taoeffect commented 9 years ago

@piroor @Daniellynet or anyone else, where is the link to the version with the fix?

It would be great if there was a link to the nightly builds in the README.md or something.

(Thanks so much for fixing this btw!)

TteokbokkiNari commented 9 years ago

@taoeffect I just downloaded the zip from here, removed a few files (compared folders/files to the current version I had), zipped it and changed the extension from .zip to .xpi. :)

taoeffect commented 9 years ago

@Daniellynet Heh, OK, maybe I will try to hold out for a more official release. Thanks though for the tip.

TteokbokkiNari commented 9 years ago

No problem! I'm just impatient and wanted to try out the fix as fast as possible, haha.

klemens commented 9 years ago

@taoeffect I have "compiled" a nightly based on 5380304aa8688e7d725a206e70188afd1a526e19 with the included make.sh buildscript, if you cannot wait :wink:: https://sync.schoelhorn.eu/f/27770b1311/?raw=1

taoeffect commented 9 years ago

Thanks @klemens, but it turned out to be remarkably simple to create the .xpi. All you need to do is clone this repo and type make. :smile:

(Thanks for making that simple @piroor!)

shearer12345 commented 9 years ago

Thanks @klemens - works great for me.

Thanks everyone for figuring it out. Thanks @piroor for making TST :-)

Miccovin commented 9 years ago

Thx for the great work - works here too!

It is possible to make an official and SIGNED update? This version is not signed so we will not be able to use it with Firefox 41 - it comes out in 4 weeks. The countdown is running...

nh2 commented 9 years ago

Can I donate to TreeStyleTab somehow? This bug reminded me that I've been using it for > 5 years and that it's quite an important tool for me.

IshmaelYavitz commented 9 years ago

I'm confused: is this bug really fixed?

I've tried both the XPI that @klemens provided; and cloning the source in git and creating the XPI manually. When I use either, clicking the star of an already-bookmarked page does open the bookmark dialog - but it's completely empty.

In other words, yes the dialog pops up, but it's still completely broken.

I'm on FF 40.0.2 on Windows 7 SP1.

klemens commented 9 years ago

It is at least fixed for me: it works both when using the star button and when using the properties of a bookmark on the bookmarks toolbar. I am using firefox 40.0.2 on a current Arch linux (4.1.6).

IshmaelYavitz commented 9 years ago

EDIT: Disregard this comment. The issue may actually be with the Add Bookmark Here extension. Bugs in both extensions are making it difficult to track down the root cause. I'll comment later if I can indeed track it to this extension.

EDIT2: Belay that. The symptoms are caused when both the Tree Style Tab and Add Bookmark Here extensions are installed. I will update the ticket that I reference below accordingly.

Pre-edit text:

I double-checked: when using a version of the extension that contains the fix, the bookmark properties dialog is still empty when I click the star. I also checked in OS X 10.10 - it doesn't work properly there either. So Firefox >= 40 on either Win7 SP 1 or OS X 10.10.

Since this ticket is about the dialog not popping up at all, and since the bookmark dialog seems to be broken in other areas of Firefox, I've opened #930: Bookmark properties dialog broken since Firefox 40.

gene-pavlovsky commented 9 years ago

I was having the same issue, with Add Bookmark Here as well. Only after I installed the latest update for ABH, as well as the nightly TST from this thread (compiled by Klemens), the bookmark feature started working again.

Frogwing commented 9 years ago

I absolutely love TST too. Using it with the latest version of Firefox but bookmarking isn't working for me either like for the rest of you. The star isn't clickable and Ctrl+D doesn't do anything so there is no comfortable way for me to edit bookmarks. Will this fix soon be added as a released fix so I can just update TST? I'm not a developer and I'm not comfortable to go in and try to custom fix things.

Thank you for this wonderful addon!

varac commented 9 years ago

I can confirm that the custom addon from https://github.com/piroor/treestyletab/issues/887#issuecomment-132389816 fixes this bug for on FF 40.0.3 (ubuntu 15.04).

nh2 commented 9 years ago

@taoeffect I have "compiled" a nightly based on 5380304 with the included make.sh buildscript

Confirming too that this fix works for me. @piroor Kindly asking for a signed upload :)

free5lot commented 9 years ago

Thank you @piroor, and please update the version at https://addons.mozilla.org/en-US/firefox/addon/tree-style-tab/

This issue was significant so the addon can get a lower rating there because of this bug. Thanks.

theoden-dd commented 9 years ago

@piroor, thank you for making fix as simple as

git clone https://github.com/piroor/treestyletab/
make

Considering more casual users I'm absolutely agree with @free5lot: please update https://addons.mozilla.org/en-US/firefox/addon/tree-style-tab/ for the good of the addon reputation.

piroor commented 9 years ago

Is this already fixed on the master/HEAD? (You can try the development build: http://piro.sakura.ne.jp/xul/xpi/nightly/ ) Then, please close this. I hope to concentrate to fix unsolved bugs.

alice0775 commented 9 years ago

I can verify to fix the problem.

varac commented 9 years ago

hugh, why was this issue closed without further explanation ? Is it fixed, and if so, is the fix included in addon that can be found on the mozialla adons download page ?

Miccovin commented 9 years ago

Is it really fixed? For me it is not - e.g. I could not open tabgroups with the installed fix. It seems I am the only one still having problems.

piroor commented 9 years ago

If you see more other bugs, please report each topic as each issue, because unclear subject issue is hard to be tracked.

mindrones commented 9 years ago

@piroor any reason why this fix hasn't been pushed to the mozilla addons site? It would be nice to see it distributed via addons updates, no?

piroor commented 9 years ago

@mindrones I uploaded the new released version to AMO at 2015-09-29 but it is awaiting editor's review. Current status is:

Queue Position: 140 of 159