Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tab Mix Plus' tab open location modification, does not work with userscripts #2365

Closed
mzso opened this issue Feb 16, 2016 · 6 comments
Closed

Tab Mix Plus' tab open location modification, does not work with userscripts #2365

mzso opened this issue Feb 16, 2016 · 6 comments
Milestone
Bankruptcy

Comments

@mzso
Copy link

mzso commented Feb 16, 2016

Hi!

I described the problem here: https://greasyfork.org/en/forum/discussion/comment/21391

Basically when that script opens a tab it's always at the end instead of next to the current tab, which I have set in TMP.
It didn't use to be like this. It got broken sometime last year.
@arantius
Copy link
Collaborator

See also #2269 . It's very difficult to get all the features to work at all on their own. Trying to let another extension further change things is unlikely to happen. If you can find TMP's author and get them to work together with us, maybe.

@arantius arantius added this to the Pony milestone Feb 16, 2016
@onemen
Copy link

onemen commented Feb 17, 2016

I'm the developer of Tab Mix Plus.

currently the incompatibility is because Tab mix fork gBrowser.addTab to move the tab to its place but GM do it in GM_openInTab because it need to move the tab after scriptTab that is not always the current tab or last related tab that addTab.

the only safe fix to this issue is to add a preference to let the user decide where to open the tab
(unless the script call GM_openInTab with {insert: ???} argument)

    var prefRel = (aMessage.data.afterCurrent === null)
        ? getBool("extensions.greasemonkey.insertAfterScriptTab")
        : aMessage.data.afterCurrent;
    if (prefRel) {
      tabBrowser.moveTabTo(newTab, scriptTab._tPos + 1);
    } else {
      tabBrowser.moveTabTo(newTab, tabBrowser.tabs.length - 1);
    }

@the8472
Copy link
Contributor

the8472 commented Feb 23, 2016

GM sets an ownerTab on the new tab, couldn't TMP just move it to the desired place after it has been created based on that information?

@onemen
Copy link

onemen commented Feb 23, 2016

GM_BrowserUI.openInTab moveis the new tab after "TabOpen" event was fired (by gBrowser.addTab)
I can't rely on "TabMove" event to be from GM_BrowserUI.openInTab

This propose patch will allow Tabmix to override the preference that GM_BrowserUI.openInTab uses to move the tab. Tabmix is using "extensions.tabmix.openTabNext"

In Tab mix code i will add this

    if (GM_BrowserUI && GM_BrowserUI.insertAfterScriptTab) {
      GM_BrowserUI.insertAfterScriptTab = "extensions.tabmix.openTabNext";
    }

GM patch

+ GM_BrowserUI.insertAfterScriptTab = "browser.tabs.insertRelatedAfterCurrent";
  GM_BrowserUI.openInTab = function(aMessage) {
    var browser = aMessage.target;
    var tabBrowser = browser.getTabBrowser();
    var scriptTab = tabBrowser.getTabForBrowser(browser);
    var scriptTabIsCurrentTab = scriptTab == tabBrowser.mCurrentTab;
    // Work around a race condition in Firefox code with E10S disabled.
    // See #2107 and #2234
    // Todo: Remove timeout when http://bugzil.la/1200334 is resolved.
    GM_util.timeout(function () {
      var newTab = tabBrowser.addTab(
          aMessage.data.url,
          {
              'ownerTab': scriptTab,
              'relatedToCurrent': scriptTabIsCurrentTab,
          });

      var getBool = Services.prefs.getBoolPref;

      var prefBg = (aMessage.data.inBackground === null)
          ? getBool("browser.tabs.loadInBackground")
          : aMessage.data.inBackground;
      if (scriptTabIsCurrentTab && !prefBg) tabBrowser.selectedTab = newTab;

      var prefRel = (aMessage.data.afterCurrent === null)
-         ? getBool("browser.tabs.insertRelatedAfterCurrent")
+         ? getBool(GM_BrowserUI.insertAfterScriptTab)
          : aMessage.data.afterCurrent;
      if (prefRel) {
        tabBrowser.moveTabTo(newTab, scriptTab._tPos + 1);
      } else {
        tabBrowser.moveTabTo(newTab, tabBrowser.tabs.length - 1);
      }
    }, 0);
  };

@mzso mzso mentioned this issue Mar 7, 2016
Closed
@mzso
Copy link
Author

mzso commented Mar 24, 2016

@arantius commented on 2016. febr. 16. 16:01 CET:

See also #2269 . It's very difficult to get all the features to work at all on their own. Trying to let another extension further change things is unlikely to happen. If you can find TMP's author and get them to work together with us, maybe.

But maybe you could check/evaluate the proposed patch by the TMP dev.

@arantius arantius modified the milestones: Pony, Bankruptcy Jul 25, 2017
@mzso
Copy link
Author

mzso commented Jul 25, 2017

Can I ask what does the Pony milestone mean? :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Bankruptcy
Development

No branches or pull requests
4 participants
@arantius @the8472 @onemen @mzso