Issue 125 - Multiple windows opening

[AWolf81]

Changes

• Save injectedTabs object {tabID1: true, tabID2: true} in browser.storage.local with key injectedTabs
• Modified check from indexOf to object-based
• Add update of local storage at every change of injectedTabs
• Update web-ext extension (without this npm start had an issue - the extension could not be installed with newer FF version.)

Tests

• [x] Open multiple tabs with file links, reload one tab and switch between tabs. Test if there are multiple Windows for folders or files after each switch. --> No multiple instances detected.
• [x] Dynamic link handling - every case of http://localhost:3000/issue1/issue.htm working. No multiple windows. Link handling OK.
• [x] Settings unaffected? Tested whitelist setting. Works. Just removing a URL requires a page reload - this could be improved later.
• [x] Trello test - Link click works, reveal not working (opens editing mode) - same behavior as the previous version. --> OK
• [x] Test normal link + file link click (see http://localhost:3000/issue113/index.html)

Fixes

125

@maxm199 would be great if you could test the branch if this is working as expected.

[maxm199]

Later I'll try this branch and then report back. Thanks

[maxm199]

Tried several times with multiple windows and never had the issue. Seems to be fine, nice job!

[AWolf81]

@maxm199 great, thanks for testing. I'll merge this today or tomorrow.

[AWolf81]

@maxm199 Issue #113 was causing the same issue. The browser's back button caused it. Clicking a regular link and then using the back button was not correctly handled - multiple events were attached.

I think it's a cache issue. The page is with the content script as it's using the cached page.

One way to improve this is to force a reload with the following lines in content.js after $.noConflict():

    var perfEntries = performance.getEntriesByType("navigation");
    if (perfEntries[0].type === "back_forward") { 
        // force reload if browser history is used --> avoid duplicated event handlers
        location.reload();
    }

This would work but I don't like this.

Another solution is to add the webNavigation permission and add the cached tab to injectedTabs. So it won't inject the page again. In background script:

       const browserHistoryHandler = (details) => {
            if (details.transitionQualifiers.includes("forward_back"))
            {
                // console.log("using cached page");
                this.injectedTabs[details.tabId] = true; // Add tabId to keep track of cached injection.
                browser.storage.local.set({ injectedTabs: this.injectedTabs });
            }
        }

        browser.webNavigation.onCommitted.addListener(browserHistoryHandler);

I tested this and it looks OK. I think it's OK to only handle back & forward transition here. The only situation where it could fail is if someone disabled browser history caching. Probably a rare case and it can be fixed by a browser reload.

[maxm199]

I can't see any issue. Unfortunately I won't be able to test it until tomorrow if you need it. Have a nice day.

[maxm199]

Just checked from remote. I can confirm that the version I tested 3 days ago had that issue, with the back button on click it opened 2/3 windows. With the last changes it opens just 1 window on back/forward button, nice catch!

[AWolf81]

OK, thanks for testing and your confirmation. Good to hear that this is working.