Tear off tab with SDK

I’ve done some testing on you methods, @kousu.

There is a serious source of issues!

After calling tab.detach(), a new window with a new tab is created, even though the view of the old tab is transferred to the new window and state is preserved. The tab gets a new id and open event is emited on sdk/tabs collection.
The new tab doesn’t catch any tab events afterwards! Neither does the old tab model.
If you access tab properties from old tab model, it still works, but after the tab is moved to the new window, before fully initialized, trying to access for example tab.url throws an error, because viewFor(tab) == undefined at some point.
I’m not sure, but there is a chance that the events are not emited for the new tab across other extensions as well.
This is unacceptable!

gBrowser.tabContainer.appendChild(tabView) is not working.
gBrowser.tabContainer is simply a DOM element, so is tabView = viewFor(sdkTab).

By moving tabView to a different window, the associated panel (tab’s browser) is not moved automatically.

In gBrowser.addTab, after appending the tab to new window, a browser is created:

this.tabContainer.appendChild(t);
// ...
b = this._createBrowser(...); 
/// ...
t.linkedBrowser = b;

Really? What Firefox version? I’m 44.0.2 on Arch Linux. It works for me.

FF 44.0.2 and 47.0a1 (Nightly) on Windows.

I’ve done some testing on you methods, @kousu.

There is a serious source of issues!

After calling tab.detach(), a new window with a new tab is created,
even though the view of the old tab is transferred to the new window
and state is preserved. The tab gets a new id and open event is
emited on sdk/tabs collection.
The new tab doesn’t catch any tab events afterwards! Neither does the
old tab model.

I don’t see that tabs.on('open'). In fact, I never see one for any first tab in a window, which seems like a bug to me but you never know.

It would be odd for moving a tab to trigger an open if its still the same tab.

Can you share your experiments as test code? Its easiest to interrogate this if each bug you’re seeing comes in a separate extension that I can jpm run, though ActionButtons embedded within a single extension would be okay.

If you access tab properties from old tab model, it still works, but
after the tab is moved to the new window, before fully initialized,
trying to access for example tab.url throws an error, because
viewFor(tab) == undefined at some point.

This is the same error I had above. The SDK docs explicitly say you can’t trust tab.url until tab.on('ready'), and while they don’t explicitly say when you can or can’t trust having viewFor(.) or view for(tab).gBrowser, I found that you need to wait as well. Just do your code in a tab.on('ready') handler and deal with it.

I’m not sure, but there is a chance that the events are not emited for
the new tab across other extensions as well.
This is unacceptable!

It’s not a new tab though so it shouldn’t be emitting these events anyway. But can you show this in a test case?

Here is the index.js file:

    // -------------------------------------------------------------
    /// Testing and playing
    // -------------------------------------------------------------
    var sw = require('./setWindow');
    // -------------------------------------------------------------
    const {
        setTimeout,
        setImmediate,
    } = require("sdk/timers");

    var TABS = require("sdk/tabs");
    var browserWindows = require("sdk/windows").browserWindows;
    // -------------------------------------------------------------

    // Marck start time
    var tmr = Date.now();

    // -------------------------------------------------------------
    /// Listeners:
    browserWindows.on('open', function (win) {
        log('~win', getWinId(win), 'open', win.tabs.length, win.tabs[0].id);
    });

    // on open tabs might be uninitialized and it is dangerous to alter it in any way
    TABS.on('open', function (tab, event) {
        log('~tab', tab.id, 'open', tab.readyState, tab.url, event?true:event);
    });

    // This one is not documented, but interesting
    TABS.on('create', function (tab, ...args) {
        log('~tab', tab.id, 'create', tab.readyState, tab.url, ...args);
    });
    TABS.on('ready', function (tab) {
        log('~tab', tab.id, 'ready', tab.readyState, tab.url);
    });
    TABS.on('load', function (tab) {
        log('~tab', tab.id, 'load', tab.readyState, tab.url);
    });
    TABS.on('move', function (tab, ...args) {
        log('~tab', tab.id, 'move', tab.readyState, tab.url, ...args);
    });

    /// Some URLs
    var urls = [
        'https://duzun.me/?window1_tab0_open_before_window',
        'https://www.google.com/?window1_tab1_detatched_to_window3',
        'https://nodejs.org/en/?window1_tab3_next_to_detatched_tab',
        'https://discourse.mozilla-community.org/t/tear-off-tab-with-sdk/7085/19/',
        'http://npmjs.org/?window2_tab1_moved_to_window1',
    ];

    /// Experiments
    var w0 = browserWindows[0];
    w0.tabs[0].url = urls[3];

    var w1 = browserWindows.open({
        url: urls[0],
        onOpen: function (w) {
            // Can't open any tab on this window until t0.readyState != 'uninitialized', so use what SDK offers - onReady,
            // even though it depends on document.readyState == 'interactive' inside tab, which could take a lot of time to happen :-(
            var t0 = w.tabs[0];
            t0.once('ready', function (t0) {
                w.tabs.open({
                    url: urls[1],
                    index: 1,
                    onOpen: function (t1) {
                        log('~t1', t1.id,'detaching', getWinId(t1.window));
                        t1.detach();

                        // This one would try to access .url multiple times, thus throws an error
                        wait4url(t1, log);
                    },
                });
                w.tabs.open({
                    url: urls[2],
                    index: 3,
                });
            });

            var t4 = w0.tabs[0];
            t4.once('ready', function (t4) {
                w0.tabs.open({
                    url: urls[4],
                    onReady: (tab) => {
                        log('~move tab', tab.id, tab.url, 'from', getWinId(w0), 'to', getWinId(w1));
                        tab.setWindow(w1);
                    },
                });
            });
        }
    });

    /// Helpers:

    function passed() {
        return Date.now() - tmr;
    }

    function log(...args) {
        console.log(passed(), '~', ...args);
    }

    function getWinId(win) {
        if ( !win ) return undefined;
        var id = win.id;
        if ( id ) return id;
        win.id = getWinId._id = id = (getWinId._id||0) + 1;
        return id;
    }

    function wait4url(tab, cb) {
        var readyState;
        return (function _wait() {
            var url = tab.url;
            // new tab's url is 'about:newtab'
            if ( (url == 'about:blank' || !url) && (readyState = tab.readyState) != 'complete' && readyState != 'interactive' ) {
                return setTimeout(_wait, 4);
            }
            cb(tab, url);
            return url;
        }())
    }

Also save your patches of Tab to setWindow.js file.

With this test the detached window emits events, but earlier today while playing with code I’ve got in a situation where it didn’t. I guess becouse of an error right after .detatch call.

setWindow() behaves as described earlier, and there is an error in the console after detatch().

You can filter console output by “~” and follow events…

Finally I’ve come up with a complete solution! :mask:

I’m not 100% sure about “detaching”, but haven’t seen issues if used after everything is initialized.

1 Like

I’ve dropped your code into my project at https://github.com/kousu/disabletabs/tree/duzun. Detaching seems to work fine, though it’s a bit counterintuitive to say .setWindow(null) to mean “make a new window” to me, so I’ve wrapped it up in a new .detach().

I’m very pleased for you! How long have you been working on this? 4 months?

Some followups:

  • Why do you return a Promise and also take a callback? Aren’t promises supposed to replace (well, adopt) callbacks?
  • Were the issues you ran into entirely solved by deferring work until .gBrowser was allocated or is there some kind of hidden Linux-Windows incompatibility?
  • how does newBrowser.docShell; make sure it has a docshell? Is that secretly invoking some C extension code, or is that just a forgotten line?
  • is // For some reason this doesn't seem to work :-( if ( selected ) { gBrowser.selectedTab = newTab; }
    necessary?
  • Instead of checking if ( gBrowser.adoptTab ) { in the handler, check it at load time and monkey-patch.
  • I really appreciate all the comments you left. The comments in the internal FF code are lacking: I keep getting confused about which data structure is which: a model tab, a view tab, a browser, the other kind of browser…

detach is not a good name, at least beacause there is tab.attach(), which attaches a script to the document, so it might be confusing.
So far I could think of setWindow(null), which to me is intuitive and lets you use same API:
setWindow(window1) moves to window1
setWindow(null||undefined||false||0||"") moves to no (existing) window -> new window.

For two reasons:

  1. callback receives the new tab and old tab’s ID, which is important to know in some cases - cb(newTab, oldTabId)
  2. callback is called synchronously when gBrowser exists, but promise is always async.

I don’t see any reason to not support callbacks, even though it returns a promise at the same time.

Actually I’m organazing the extension code in such a way that it interacts with tabs and windows only after initialization. I’m still testing and adjusting the extension, so if I have any issues, I’ll update setWindow's code. So far everything looks good and no issues (I’m not using “detach” in my extension, yet).


To answer the rest:

  • gBrowser.adoptTab is not defined in FF44 and I don’t want to touch “future” methods, as this might affect other libraries. But I’ll do the check at load time.

  • selectedTab didn’t work at some point, but I have to retest it with the final version, it might work now. Anyways it is a minore issue (my extension takes care of this by itself :slight_smile: ).

  • A lot of properties in FF code are getters and setters and some of them get initialized on first call, so is docShell. Because the loading of browser stops, docShell might never get initialized.

I also get confused by what is what in FF code. The documentation is not exhaustive and I’m doing a lot of guessing and testing for every API method.

I think this discussion might be interesting:

Bug 1214007 - Implement chrome.tabs.move

I’ve updated the code for setWindow(null).
gBrowser.replaceTabWithWindow returns the new window, not the new tab, so I had to update the code to wait for window to open, then return model for new tab.

How is it working @kousu ? Did you test it?
On my side it looks good so far.

I haven’t put it through its paces. With the previous tear off method, a bunch of scrap about:blank pages are made that you can see if you find the right nook of Developer Tools. It sounds like your update should fix that though.

This was an excellent open source experience! Thanks for building on this @DUzun and @kousu. If I ever need this, I will use the method you two collaborated on rather then mine. Long live open source!

Please do share once you put it through the ringer and see how it behaves @kousu