piroor / newtabfromlocationbar

Opens new tabs from the location bar, by conditions.
http://piro.sakura.ne.jp/xul/newtabfromlocationbar/
9 stars 2 forks source link

Not a Bug, Not a Request: Workaround Idea #18

Closed joshuacant closed 6 years ago

joshuacant commented 6 years ago

Hi Piro, it looks like we've both been working on this issue. You are smarter than I am, so maybe you can put what I've found to good use. I have been working with browser.webRequest.onBeforeRequest and have had some pretty good luck. However, I've hit a wall in terms of what can be done. Maybe by mixing your browser.webNavigate.onCommitted method with mine would get closer to perfect?

Here's the method I've come up with that seems to work 100% of the time, but also opens new tabs for some URLs it shouldn't. Here's why (I think) it works: When entering a new URL from the address bar, documentUrl and originUrl are always blank, the type is always 'main_frame' and the tabId is never "-1" (I don't know what -1 represents, but it seemed to be navigation requests generated in the background by Firefox itself).

If there's anything you can accomplish with this, please do. I'm not likely to be able to do it myself, I'm too new to webextensions and javascript. Thanks again for all you do.

background.js:

var newTabUrl = null;

function logURL(requestDetails) {
  // Attempt to filter to address bar only (NOT PERFECT).
  if (!requestDetails.documentUrl && !requestDetails.originUrl 
      && requestDetails.type == 'main_frame' && requestDetails.tabId != -1) {

    // New request matches the tab we just created, don't create another new tab.
    if (requestDetails.url.split('//')[1] == newTabUrl) { 
      //console.log('url matched');
      //console.log(requestDetails);
      return {cancel: false};
    }

    // New request actually looks to be from address bar. Cancel request and create new tab.
    console.log('address bar, probably');
    console.log(requestDetails);
    browser.tabs.create({url:requestDetails.url});
    newTabUrl = requestDetails.url.split('//')[1];
    return {cancel: true};
  }
  else {
    //console.log('everything else');
    //console.log(requestDetails);
    return {cancel: false};
  }
}

browser.webRequest.onBeforeRequest.addListener(logURL,{urls: ["<all_urls>"]},["blocking"]);

// Bugs:
// 30X HTTP redirects aren't detected correctly, too many new tabs created
// Links opened from outside the browser aren't detected correctly, creates an extra blank tab

manifest.json:

{
  "manifest_version": 2,
  "name": "Address Bar New Tab Test",
  "version": "0.0.1",
  "description": "description",
  "homepage_url": "http://domain.tld",
  "icons": {
    "96": "icons/icon-96.png"
  },

  "permissions": [
    "webNavigation",
    "webRequest",
    "webRequestBlocking",
    "tabs",
    "<all_urls>"
  ],

  "background": {
    "scripts": [ 
      "js/background.js" 
    ]
  }
}
piroor commented 6 years ago

Thanks! I've wrote new mode based on webRequest. I didn't mention that documentUrl and originUrl are available to detect loading from the location bar. You can try the new development version at http://piro.sakura.ne.jp/xul/xpi/nightly/newtabfromlocationbar-we.xpi

On https://bugzilla.mozilla.org/show_bug.cgi?id=1394304 you wrote:

I have a webextension using browser.webRequest.onBeforeRequest that doesn't have the page reload problem, but does have problems with links from external sources and HTTP 30X redirects.

Currently both those cases seem to work well for me with the default configuration. However I think still there are some hidden problems. For example I don't test well on the private browsing mode. To activate this on a new release version, I need more doogfooding.

piroor commented 6 years ago

Here is failure cases I've found:

joshuacant commented 6 years ago

I just installed your nightly build and have been playing around. I ran into a weird case where a new tab wasn't opened after I typed a new URL, then when I hit the back button, that opened a new tab with the previous page instead.

I think there will be many such edge cases until Mozilla approves official API. :( But something is better than nothing!

joshuacant commented 6 years ago

I can't reproduce the problems I was having with 30X redirects or external links with your version either. You must have fixed them by some other part of your code.

joshuacant commented 6 years ago

After 24 hours of disabling legacy Tab Mix Plus and using this, it's actually quite good. But not perfect. Things are inconsistent. It's strange. Yesterday night I was not getting the extra tabs for external links (clicking a link in another program on PC) but today I've seen it a couple times. I've also seen the "new tab not created, hit back, new tab is created" issue a second time.

Do you think it'd be worth adding a very verbose logging to file feature? (Actually, is that even possible with webextensions?) Then over time these edge case issues would form patterns that could be addressed.

piroor commented 6 years ago

If you activate "Debug mode" at this addon's configurations, you'll see detailed information when the loading request is redirected to a new tab.

piroor commented 6 years ago

A new failure case I've found:

  1. Prepare a bookmark with a URI about:blank.
  2. Open new tab with a specific URL like https://github.com/
  3. Load about:config to the tab from bookmark. Then the tab has two session histories: https://github.com/ and about:config.
  4. Set browser.tabs.loadBookmarksInTabs to false. (default value)
  5. Click the "Back" button.
    • Expected result: the tab goes back to https://github.com/
    • Actual result: a new tab for https://github.com/ is opened.

Log:

newtabfromlocationbar<BG>:    onBeforeRequest loading on existing tab  common.js:21:78
newtabfromlocationbar<BG>:    tab  Object { url: "about:config", newTab: false, previousUrl: "http://piro.sakura.ne.jp/" }  common.js:21:78
newtabfromlocationbar<BG>:     tryRedirectToNewTab Object { requestId: "82127", url: "http://piro.sakura.ne.jp/", originUrl: undefined, documentUrl: undefined, method: "GET", type: "main_frame", timeStamp: 1511669556217, frameId: 0, parentFrameId: -1, proxyInfo: null, 他 2 個... }  common.js:21:78
newtabfromlocationbar<BG>:     Redirect to new tab

Details of onBeforeRequest:

{"requestId":"82127","url":"http://piro.sakura.ne.jp/","method":"GET","type":"main_frame","timeStamp":1511669556217,"frameId":0,"parentFrameId":-1,"proxyInfo":null,"ip":null,"tabId":976}
piroor commented 6 years ago

Different failure case:

  1. Prepare a bookmark with a URI under addons.mozilla.org, ex. https://addons.mozilla.org/developers/
  2. Set browser.tabs.loadBookmarksInTabs to false. (default value)
  3. Open new tab with a specific URL like https://github.com/
  4. Load https://addons.mozilla.org/developers/ to the tab from bookmark.
    • Expected result: a new tab for https://addons.mozilla.org/developers/ is opened and the current tab stays at https://github.com/
    • Actual result: a new tab for https://addons.mozilla.org/developers/ is opened and the current tab goes to https://addons.mozilla.org/developers/ (so you'll see two tabs with same URI)

Log and details:

newtabfromlocationbar<BG>:    onBeforeRequest loading on existing tab  common.js:21:78
newtabfromlocationbar<BG>:    tab  Object { url: "http://piro.sakura.ne.jp/", newTab: false, previousUrl: null }  common.js:21:78
newtabfromlocationbar<BG>:     tryRedirectToNewTab Object { requestId: "83996", url: "https://addons.mozilla.org/ja/devel...", originUrl: undefined, documentUrl: undefined, method: "GET", type: "main_frame", timeStamp: 1511670207950, frameId: 0, parentFrameId: -1, proxyInfo: null, 他 2 個... }  common.js:21:78
newtabfromlocationbar<BG>:     Redirect to new tab
{"requestId":"83996","url":"https://addons.mozilla.org/ja/developers/","method":"GET","type":"main_frame","timeStamp":1511670207950,"frameId":0,"parentFrameId":-1,"proxyInfo":null,"ip":null,"tabId":984}

This is due to limitations of WebExtensions APIs. If you have a preference value privacy.resistFingerprinting.block_mozAddonManager=true, this problem won't happen. See also: https://www.ghacks.net/2017/10/27/how-to-enable-firefox-webextensions-on-mozilla-websites/

joshuacant commented 6 years ago

I think I tracked down the failure case I've been seeing intermittently:

  1. Create a bookmark with the %s substitution variable and assign a keyword, or use the default address bar search.
  2. Execute the search and click any link on the results page.
  3. Press the back button.

Expected result: Tab goes back to the initial results page. Actual Result: A new tab is created with the results page.

Looking at the debug objects, I can't see any way to tell these webRequests from actual location bar entries. I think Firefox is "re-playing" their creation webRequest somehow, so they look like location bar entries to the addon.

Workaround possible? Monitor for back button use and prevent the addon from creating the new tab? Keep a history of new tabs created and filter out duplicates (could have side effects)?

joshuacant commented 6 years ago

Another pair of failure cases (don't know how important these are, but they deserve mentioning):

  1. Open a tab to any normal website url.
  2. Enter any about:_____ url in the location bar (ie: about:addons, about:debugging, etc)

Expected: New tab is created. Actual: New tab is not created.

  1. Open a tab to any about:_____ url in the address bar (ie: about:addons, about:debugging, etc)
  2. Enter any normal website url in the location bar.

Expected: New tab is created, old tab is preserved. Actual: New tab is created, old tab is broken: Contents gone, shows only solid grey background.

piroor commented 6 years ago

Cases about:* looks same to my case https://github.com/piroor/newtabfromlocationbar/issues/18#issuecomment-346982922

I've researched about your "Back" case https://github.com/piroor/newtabfromlocationbar/issues/18#issuecomment-347031322 and I've realized that onBeforeRequest listener is called when I back to the initial page of the tab. I don't know "why", but it seems a condition to detect to-be-ignored case.

joshuacant commented 6 years ago

Failure case:

  1. Open an email inside GMail that has a URL and click it.

Expected: A single new tab is created with the final desination URL loaded. Actual: 1 new tab (blank contents) is created for the redirect URL (normally something like: https://mail.google.com/_/scs/mail-static/_/js/k...etc...), and 1 new tab is created for the desired destination URL.

I clicked multiple times and got 2 different debug results. I think it has to do with caching? Here's the non-cached debug output:

newtabfromlocationbar<BG>:  onBeforeRequest loading on existing tab  Object { url: "https://github.com/piroor/newtabfro…", initialUrl: "https://github.com/piroor/", newTab: false, active: true, windowId: 3, previousUrl: "https://github.com/piroor/newtabfro…" }  common.js:21:78
newtabfromlocationbar<BG>:  tryRedirectToNewTab Object { requestId: "19742", url: "https://www.google.com/search?q=wyc…", originUrl: undefined, documentUrl: undefined, method: "GET", type: "main_frame", timeStamp: 1511890649093, frameId: 0, parentFrameId: -1, proxyInfo: null, 2 more… }  common.js:21:78
newtabfromlocationbar<BG>:  Redirect to new tab

And here's the cached debug output:

newtabfromlocationbar<BG>: onBeforeRequest loading on existing tab  Object { url: "wyciwyg://6/https://mail.google.com…", initialUrl: "wyciwyg://6/https://mail.google.com…", newTab: false, active: true, windowId: 3, previousUrl: "about:blank" }  common.js:21:78
newtabfromlocationbar<BG>: tryRedirectToNewTab Object { requestId: "19553", url: "https://www.google.com/url?hl=en&q=…", originUrl: undefined, documentUrl: undefined, method: "GET", type: "main_frame", timeStamp: 1511890440439, frameId: 0, parentFrameId: -1, proxyInfo: null, 2 more… }  common.js:21:78
newtabfromlocationbar<BG>: Redirect to new tab