xiaoxiaoflood / firefox-scripts

userChromeJS / autoconfig.js and extensions
Mozilla Public License 2.0
1.02k stars 86 forks source link

Extension Options Menu Issue with Tab Mix+ #114

Closed Tanookirby closed 2 years ago

Tanookirby commented 2 years ago

I have been using your Extension Options Menu script for a while now, and I noticed that with Tab Mix Plus, clicking the extension name on the list opens and then quickly closes a tab. I'm guessing that TMP causes issues since it uses a small window for its options. Would this be something that could be looked at, or is this a Tab Mix Plus issue?

xiaoxiaoflood commented 2 years ago

Yes, this is in TMP side.

Fx 59 removed the ability for extensions to open prefs in its own window, so I implemented a workaround in legacy extensions to open options in tab, then the content will notice it was opened in tab and automatically reopen itself in window then close the tab. This workaround was also implemented in TMP.

But in 2021 @117649 found a way to restore original behavior, which is way better than the ugly workaround we were using. So I removed the workaround from my legacy addons, letting BootstrapLoader to handle the opening of prefs window as Fx did natively up to v58. But TMP still uses the workaround.

@onemen, my suggestion is to remove https://github.com/onemen/TabMixPlus/blob/a4f462fefff56c518bccabd832e8eaa82119cbc9/addon/chrome/content/preferences/preferences.js#L782-L813 and restore optionsType 1 (AddonManager.OPTIONS_TYPE_DIALOG) in install.rdf instead of 3 (AddonManager.OPTIONS_TYPE_TAB).

onemen commented 2 years ago

@onemen, my suggestion is to remove https://github.com/onemen/TabMixPlus/blob/a4f462fefff56c518bccabd832e8eaa82119cbc9/addon/chrome/content/preferences/preferences.js#L782-L813 and restore optionsType 1 (AddonManager.OPTIONS_TYPE_DIALOG) in install.rdf instead of 3 (AddonManager.OPTIONS_TYPE_TAB).

@xiaoxiaoflood, the result of your suggestion is the Tab Mix preferences opens in a tab instead of a dialog window, but this is not supported at the moment

117649 commented 2 years ago

@onemen, my suggestion is to remove https://github.com/onemen/TabMixPlus/blob/a4f462fefff56c518bccabd832e8eaa82119cbc9/addon/chrome/content/preferences/preferences.js#L782-L813 and restore optionsType 1 (AddonManager.OPTIONS_TYPE_DIALOG) in install.rdf instead of 3 (AddonManager.OPTIONS_TYPE_TAB).

@xiaoxiaoflood, the result of your suggestion is the Tab Mix preferences opens in a tab instead of a dialog window, but this is not supported at the moment

Its set optionsType to 1.
3 is open in a tab. TMP use same basicly logic but a standalone script to achieve dialog open in Add-on manager page. Switch optionsType to 1 shouldn't affect that mostly(I hope). Plus you need update bootstrap loader to a fairly newer version.

xiaoxiaoflood commented 2 years ago

the result of your suggestion is the Tab Mix preferences opens in a tab instead of a dialog window

Unless you're using a very outdated BootstrapLoader.jsm, as I explained my suggestion will restore prefs in dialog window without the needing of the ugly workaround I wrote years ago and still exists in TMP code, causing this issue here.

onemen commented 2 years ago

I have all the latest files from @xiaoxiaoflood firefox-scripts

I have remove the workaround from: https://github.com/onemen/TabMixPlus/blob/a4f462fefff56c518bccabd832e8eaa82119cbc9/addon/chrome/content/preferences/preferences.js#L782-L813 and changed in install.rdf:

-    <em:optionsType>3</em:optionsType>
+    <em:optionsType>1</em:optionsType>

Tab mix preference open in dialog correctly from Tab Mix code:

However when I am trying to use Options Menu script it open in new tab instead.

@117649 can you test it and see if i missed something ?

onemen commented 2 years ago

OK, IT WORK

tested again on a new profile and it work, look like there are some cache issue in my development profile

117649 commented 2 years ago

OK, IT WORK

tested again on a new profile and it work, look like there are some cache issue in my development profile

image

@onemen,@xiaoxiaoflood It should be work if you just shutdown all Firefox process completely. This because how bootstraploader.js is working. When each time the addon is startup a new sandbox is created for that instance. However there is no logic to force dispose the old sandboxes. Thus if (un)lucky which is quite usual they would lingering forever. A soft restart undertake by Firefox itself is just not enough to banish them. Including the 'Clear startup cache...' on about:support.

That's what this issue really want to talk about. But I guess it would be just dealt by some tip text since most bootstrap addons request a restart by default.

onemen commented 2 years ago

now that i've set optionsType to AddonManager.OPTIONS_TYPE_DIALOG, when Tab Mix enabled for the first time and the i've click on the menu to open the options dialog, i see the error:

Uncaught TypeError: can't access property "hidden", this.querySelector(...) is null
    <anonymous> chrome://userchromejs/content/BootstrapLoader.jsm:50
    update chrome://mozapps/content/extensions/aboutaddons.js:3355
    connectedCallback chrome://mozapps/content/extensions/aboutaddons.js:2956
    insertCardInto chrome://mozapps/content/extensions/aboutaddons.js:4047
    _updateAddon chrome://mozapps/content/extensions/aboutaddons.js:4214
    update chrome://mozapps/content/extensions/aboutaddons.js:4140
    withCardAnimation chrome://mozapps/content/extensions/aboutaddons.js:4163
    update chrome://mozapps/content/extensions/aboutaddons.js:4138
    handleEvent chrome://mozapps/content/extensions/aboutaddons.js:4290
BootstrapLoader.jsm:50:14

from here: https://github.com/xiaoxiaoflood/firefox-scripts/blob/291bebc4f2edc919dd273f2cecbbfda61e157695/chrome/utils/BootstrapLoader.jsm#L47-L51

xiaoxiaoflood commented 2 years ago

What are the steps to reproduce please? I downloaded v1.0.0-pre.4, changed optionsType value to 1, removed the workaround from preferences.js, installed extension with about:addons open, clicked TMP Options and the dialog opened flawlessly. Fx 97.

onemen commented 2 years ago

What are the steps to reproduce please? I downloaded v1.0.0-pre.4, changed optionsType value to 1, removed the workaround from preferences.js, installed extension with about:addons open, clicked TMP Options and the dialog opened flawlessly. Fx 97.

I can also trigger the issue by disable Tab mix and enable it again.

xiaoxiaoflood commented 2 years ago

Thanks. I think I got it. Try replacing https://github.com/xiaoxiaoflood/firefox-scripts/blob/291bebc4f2edc919dd273f2cecbbfda61e157695/chrome/utils/BootstrapLoader.jsm#L50

by

this.querySelector('panel-item[data-l10n-id="preferences-addon-button"]').hidden = false;

Confirm it's working and I'll push the update.

After updating BootstrapLoader.jsm, you probably need to delete startup-cache folder in your profile folder to apply the change.

onemen commented 2 years ago

Look good to me 👍

onemen commented 2 years ago

you need to verify that this patch also work on older Firefox versions

xiaoxiaoflood commented 2 years ago

This selector is valid for ESR91 - the oldest version supported by Mozilla -, so it's safe. https://searchfox.org/mozilla-esr91/source/toolkit/mozapps/extensions/content/aboutaddons.html#104

onemen commented 2 years ago

It was my fault after all , i've just remember that i was removing action attribute it the past

https://github.com/onemen/TabMixPlus/blob/1e80588a64253ce8317998c548de271fd49f910e/addon/chrome/content/preferences/overlay/aboutaddons.js#L13-L18

onemen commented 2 years ago

This issue was fixed by c9a86507

Try latest Tab Mix version