Closed ysard closed 6 years ago
I replaced openPopup with openPopupAtScreen, the positioning seems better on KDE.
Yes, clonedMenu seems to be a ugly workaround, but without documentation we can't explain the value of this code.
The commit where it has been introduced is here: https://github.com/nmaier/mintrayr/commit/153ccbb7ca93788e9bf820c46ed0f40a582fd18d
Developer comment: Removed the windowState hack in favor of a less invasive (but still very nasty) hiddenWindow hack.
=> It is not surprising if it doesn't work anymore...
Wrong position was fixed by last commit you made. So this is not problem anymore. Good work 👍
Windows 7 and Windows 10 is behaving the same.
Thunderbird main window is opened = menu is placed in wrong place but works.
Thunderbird main window is minimized into tray = menu will not appear.
Menu needs main window to be present in order to be displayed (on Windows). If main window is not present (eg. is minimized to tray) then menu can't be displayed.
Propably the weird code (clonedMenu, iframe/hidden.xul) was solution to this problem. Now is question how to do it without "showPopup()". We need to display "clonedMenu" instead of "menu". But clonedMenu does not have openPopup/openPopupAtScreen 😞
Ok so it's maybe a problem with the clone method...
Clone works but this line document.documentElement.appendChild(clonedMenu);
will remove "openPopup" capability. If I remove this line then "openPopup" will not fail. Menu will not appear because it is not attached to anything ofcourse.
It looks like something is wrong with "document" (Services.appShell.hiddenDOMWindow.document).
Correction - "document" is iframe (hidden.xul).
iframe is created with this:
var localS = '<?xml version="1.0"?>\
<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"></window>';
frame.src = "data:text/html;charset=utf-8," + escape(localS);
//frame.setAttribute("src", "chrome://mintrayr/content/hidden.xul");
hDocument.documentElement.appendChild(frame);
Maybe iframe is not created correctly?
Yes the naming is disorientating...
Looks like this was removed https://bugzilla.mozilla.org/show_bug.cgi?id=1145470 (hiddenDOMWindow did have some exception for loading iframes but it was removed, this is why it fails with security warning now)
And variable to allow xul iframes was removed? https://bugzilla.mozilla.org/show_bug.cgi?id=1460685
So now iframes can't load chrome/xul anymore?
Maybe but the workaround with localS seems to work well...
When I use adoptNode instead of importNode, the menu loses the methods we are interested in ...
Once our elements are detached from their parents it seems impossible to be able to do something
You are right. iframe is created just fine and even has correct content (<window...). But it is text/html instead of text/xul. We calling XUL methods on HTML elements? This is maybe the problem?
We taking XUL elements and converting them to HTML? This is why they do not have any XUL methods?
I don't think so, if i create a contentType text/xul:
frame.src = "data:text/xul;charset=utf-8," + escape(localS);
When clicking on the icon it offers me to download the content of the iframe ><
I think that the problem is above:
Do not raise an exception:
var clonedMenu = menu.cloneNode(true);
document.importNode(clonedMenu);
clonedMenu.openPopupAtScreen(x, y, true);
Raise an exception:
var clonedMenu = document.importNode(menu, true)
clonedMenu.openPopupAtScreen(x, y, true);
While the syntaxes are supposed to be similar... (cf example: https://pawelgrzybek.com/cloning-dom-nodes-and-handling-attached-events/ )
And just after the first case, this code raises an exception:
document.documentElement.appendChild(clonedMenu);
clonedMenu.openPopupAtScreen(x, y, true);
JavaScript is a wonderful language...
We need appendChild. ImportNode just copies element. So after "import" node is copied but it is not attached not anything. Don't know why the name is "import". It does not import anything to anywhere ._.
Maybe it is imported into document but not used anywhere? It's just floating somewhere.
The Document object's importNode() method creates a copy of a Node or DocumentFragment from another document, to be inserted into the current document later.
The imported node is not yet included in the document tree. To include it, you need to call an insertion method such as appendChild() or insertBefore() with a node that is currently in the document tree.
But appendChild()/insertBefore() destroys any usefulness of the element. Because? HTML? Maybe?
"hidding menu when window is minimized" is intended behaviour: https://bugzilla.mozilla.org/show_bug.cgi?id=435848
9 years back a ticket was opened to solve this issue but no progress was done: https://bugzilla.mozilla.org/show_bug.cgi?id=502457
I'm out of ideas. Maybe revert to original hack? Before 153ccbb7ca93788e9bf820c46ed0f40a582fd18d?
Ok... thank you for the search.
Maybe the revert could do the trick yes; but currently I do not have access to a Windows machine to test that :/
Very little was changed on JS side. All magic is happening in Windows dll it seems. I did not port few lines. They seems like refactoring than actual change. Windows dll did not have any changes since 153ccbb7ca93788e9bf820c46ed0f40a582fd18d so I did not revert any other changes than this hack.
Tested on Windows 10 and 7 and it seems to work just fine. Menu will appear even when minimized.
Can you test it on Linux? Or if you can please test it on another Windows machine. Since nothing was really changed it should work just fine for Linux. Source: https://github.com/kolinger/mintrayr/tree/menu_hack_revert, or XPI: mintrayr-1.4.4.zip
Should I send a PR?
Wow fantastic ! :1st_place_medal: It's ok on KDE, i will test on another Windows 7 tomorrow.
Yes you can do a PR ;)
For the past few days I did not find any problems. Works as expected. So this issue is fixed and can be closed.
Thank you for cooperation!
Same thing for me, well played :)
This fix is not on 1.4.6 release, right?
This correction is present since version 1.4.5 and 651ada7
@ysard because I have 1.4.6dev installed but still having the same issue https://mion.yukirin.xyz/images/2018/10/20/2018-10-20_08-10-33.gif
@skeith : It's a shame, I thought this problem was solved :( Thank you for the bug report; however I would prefer to focus on a new tool much more maintainable and less time-consuming than this one...
Is that tool still has minimize to tray?
Let's say that Thunderbird 63 (release date not planned) includes APIs to develop this kind of addon :)
If I may add The extension works fine on Windows 7, the bug I reported above is for Win 10 (I'll pull the version soon)
I'm on Windows 7 and for me the current version is not working properly. The context menu does not open, unless the Thunderbird window is maximized. When it's minimized to tray already, the popup will not show up. Furthermore, I'm on a setup with two screens, having the Windows taskbar and the tray area on the left screen. The popup show up in the bottom right corner of the right screen, however, where there is no taskbar or tray icon, which is... a bit awkward.
Windows 8.1 x64 SingleLanguage Thunderbird 60.2.1 (20180930223627) MinimizeToTray Reanimated 1.4.4
Context menu in tray not work.
@skeith : I will test as soon as possible on Windows 10, but it must be said that only a rewrite of the addon with a recent library will be necessary to fix all these problems.
@freejoins & @nepa : Yes it is fixed since 1.4.5. The xpi is available here: https://github.com/ysard/mintrayr/releases Because of some slowness in the verification process on AMO, I have not released a new version there yet.
@ysard: Thank you! Will you eventually release the update officially on mozilla.org? If the verification process is that slow, isn't that a reason to publish the update even earlier instead of omitting that step altogether? ;-)
This is the following of what has been written here: https://github.com/nmaier/mintrayr/issues/188#issuecomment-420934976
openPopup() is executed. But it does nothing without any error but: