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.

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

Vimium on new-tab page #1515

Closed
smblott-github opened this issue Mar 5, 2015 · 24 comments
Closed

Vimium on new-tab page #1515

smblott-github opened this issue Mar 5, 2015 · 24 comments

Comments

@smblott-github
Copy link
Collaborator

When creating a new tab with Ctrl-T, Vimium is not active and the focus is locked in the address bar. The whole Vimium UX is broken.

  • Does anyone know how to fix this?

Absent a Vimium fix, I've been using New Tab Redirect to redirect chrome://newtab to a blank page with a very-long TTL (on which Vimium is active). Unfortunately, we can't do the same trick in Vimium. First, it completely disables chrome://newtab (and it wouldn't be possible to have an option to re-enable it). Second, redirecting to pages/blank.html still leaves the focus in the address bar.

As a super lightweight alternative to New Tab Redirect, I implemented this:

(which might be of interest to some Vimium users, absent a Vimium solution).

@nikess
Copy link

nikess commented Mar 10, 2015

@smblott-github
Copy link
Collaborator Author

It'll Vimium will never work with Momentum, I think, because the new-tab URL is:

chrome-extension://laookkfknpbbblfpciffpaejjkokdgca

... and Chrome doesn't run other extensions on such pages.

@leoj3n
Copy link

leoj3n commented Mar 11, 2015

My experience is different than yours.

Setting the new tab page to pages/blank.html in Vimium Options and pressing t opens a new tab but does not leave focus on address bar. I can immediately press o, shift+j or any other Vimium shortcut.

Is your plugin really necessary? Doesn't Chrome's native new tab setting suffice?

@smblott-github
Copy link
Collaborator Author

Setting the new tab page to pages/blank.html in Vimium Options and pressing t opens a new tab but does not leave focus on address bar

@leoj3n. You're right, for tabs opened with t. My extension then gives the same effect for tabs opened with Ctrl-T.

@leoj3n
Copy link

leoj3n commented Mar 11, 2015

Ohhh got ya.

@wadkar
Copy link
Contributor

wadkar commented Mar 12, 2015

I think I might need to open a new issue - but it appears that only I am facing it.
If I put pages/blank.html in the vimium settings, it opens a search result with pages/blank.html as query. I need to use chrome-extension://aaklmmoijnlnbeogmilbplamkggababp/pages/blank.html to actually get the blank.html page from vimium's extension source.
Any suggestions/hints on looking for solution/bugfix?

@mrmr1993
Copy link
Contributor

The docs for chrome.tabs.create say (of the url parameter, which we pass as the contents of this setting):

Relative URLs will be relative to the current page within the extension.

If this isn't working properly, it sounds like a Chrome bug. Have you tried restarting the browser/reloading Vimium, to see if that makes a difference?

@smblott-github
Copy link
Collaborator Author

@mrmr1993. We can wait to see what @wadkar reports. But we could also just convert pages/blank.html to an absolute URL, and not worry about it.

@mrmr1993
Copy link
Contributor

I don't think this is something we need to consider unless this is an issue for multiple people. But it would be tidier UX to just resolve relative urls manually (like chrome says it should), if this is an issue. A chrome-extension url looks kinda scary for people who aren't familiar with them.

@wadkar
Copy link
Contributor

wadkar commented Mar 14, 2015

Test Report
Methodology: Open chrome, load the extension, ensure appropriate settings, run tests, remove the extension

  • Test 1 with Vimium from chrome store (v1.49): Happy Path 1
    1. Chrome home page set to about:blank and vimium new tab URL set to pages/blank.html
    2. Open google.com in normal window, press t, a new tab with chrome-extension://.../pages/blank.html opens
    3. Open an incognito window, press t, a new tab with chrome-extension://.../pages/blank.html opens in the non-incognito window
      (Issue Open new tab in incognito mode should open new tab in same window #1507 reported , patch submitted PR Fix #1507 Open new tab in incognito mode #1508 and accepted)
  • Test 2 with Vimium from local with PR#1508 applied: Happy Path 2
    1. Chrome home page set to about:blank and vimium new tab URL set to chrome://newtab
    2. Open google.com, press t, a new tab with chrome://newtab opens
    3. Open an incognito window, press t, a new tab with about:blank opens in the incognito window
  • Test 3 with Vimium from local with PR Fix #1507 Open new tab in incognito mode #1508 applied: Happy Path 3
    1. Chrome home page set to about:blank and vimium new tab URL set to chrome-extension://.../pages/blank.html
    2. Open google.com, press t, a new tab with chrome-extension://.../pages/blank.html opens
    3. Open an incognito window, press t, a new tab with chrome-extension://.../pages/blank.html opens in the incognito window
  • Test 4 with Vimium from local with PR Fix #1507 Open new tab in incognito mode #1508 applied: Failure Path
    1. Chrome home page set to about:blank and vimium new tab URL set to pages/blank.html
    2. Open google.com in normal window, press t, a new tab with pages/blank.html does not open
      Instead, I see search result for the query pages/blank.html
    3. Open an incognito window, press t, a new tab with search result for query pages/blank.html opens in the incognito window

Any other testing suggestions? My js-foo is a bit rusty, so I didn't attempt full debugging of PR #1508 . If you have any suggestions/hints I will be happy to give it another shot to PR #1508 . Till then, I think we should block PR #1508.

Cheers,
-S

@wadkar
Copy link
Contributor

wadkar commented Mar 14, 2015

After little bit of digging around, it appears that createNewUrlInTab calls Utils.convertToUrl
A simple fix is to test for pages/blank.html in the convertToUrl function as follows:

  convertToUrl: (string) ->
    string = string.trim()

    # Special-case about:[url], view-source:[url] and the like
    if Utils.hasChromePrefix string
      string
    else if Utils.isUrl string
      Utils.createFullUrl string
    else if string is "pages/blank.html"
      string
    else
      Utils.createSearchUrl string

This may or may not be the right way to go. Note that the fix introduces an extension specific string literal "pages/blank.html" in the middle of Utils library.

If this is acceptable then let me know and I will create another issue/PR with the fix.

Cheers,
-S

@smblott-github
Copy link
Collaborator Author

Thanks for detective work, @wadkar. With the fix you propose, typing pages/blank.html into the vomnibar would take you to the blank page. That seems wrong.

A better alternative would be to find where the new tab setting is used (here), and replace it with the full URL directly. This is the only place where pages/blank.html has a special meaning.

@mrmr1993
Copy link
Contributor

Changing

createTab: (callback) -> openUrlInNewTab { url: Settings.get("newTabUrl") }, callback

to

createTab: (callback) -> openUrlInNewTab {
  url: Settings.get("newTabUrl")
  rawUrl: true
}, callback

and

openUrlInNewTab = (request, callback) ->
  chrome.tabs.getSelected null, (tab) ->
    tabConfig =
      url: Utils.convertToUrl request.url
      index: tab.index + 1
      selected: true
      windowId: tab.windowId
    # FIXME(smblott). openUrlInNewTab is being called in two different ways with different arguments.  We
    # should refactor it such that this check on callback isn't necessary.
    callback = (->) unless typeof callback == "function"
    chrome.tabs.create tabConfig, callback

to

openUrlInNewTab = (request, callback) ->
  chrome.tabs.getSelected null, (tab) ->
    chrome.tabs.create {
      url: if request.rawUrl then request.url else Utils.convertToUrl request.url
      index: tab.index + 1
      selected: true
      windowId: tab.windowId
    # FIXME(smblott). openUrlInNewTab is being called in two different ways with different arguments.  We
    # should refactor it such that this check on callback isn't necessary.
    }, -> callback?()

should achieve exactly what we want (and IMO makes openUrlInNewTab easier to understand as a chrome.tabs.create call). We could also change openOptionsPageInNewTab to use openUrlInNewTab {url: "pages/options.html", rawUrl: true} too, to avoid any chrome.tabs.create duplicated logic. I can't make a PR for this at the moment, unfortunately.

@wadkar
Copy link
Contributor

wadkar commented Mar 14, 2015

Yep, I agree Utils isn't the right way to go. Interestingly, the fix passes Test 2 completely (new tab opens in incognito window) but fails Test 3 by opening chrome-extension://.../pages/blank.html in the non-incognito window . I thought about fixing it some other way and arrived at a solution quite very much like @mrmr1993 . Note this doesn't fix #1507 in fact it breaks it by design.

I think we should create a separate issue/PR for this and link #1507 #1508 against it. I am making a separate branch for this and testing @mrmr1993 's suggestion - stay tuned.

Cheers,
-S

wadkar added a commit to wadkar/vimium that referenced this issue Mar 14, 2015
PR philc#1508 introduced another issue that setting vimium's `newTabURL` to
the inbuilt `pages/blank.html` resulted in opening a new tab with search
query set to `pages/blank.html`.

This commit solves the issue by conditionally calling `Utils.convertToUrl`.
Also, it changes `openOptionsPageInNewTab` call to use
`openUrlInNewTab` for consistency and reduce usage of
`chrome.tabs.create` API call.

Note that this may not solve philc#1507 as chrome seems to open
`chrome-extension://...` URLs in _non-incognito_ window *by design*.
You may want to set it to (some other blank page)[1] so that it can be
accessed from incognito mode. You can always open `chrome://newtab` from
incognito window.

[1] http://this-page-intentionally-left-blank.org/
@wadkar
Copy link
Contributor

wadkar commented Mar 14, 2015

So, I followed through @mrmr1993 's changes and created PR #1532 . It certainly fixes some issues (setting aside incognito mode).

To be honest, I am not sure as to why Test 1 above should ever pass for incognito windows. I don't suppose chrome allows opening of chrome-extension://... URL in an incognito window. Chrome seems to redirect it to non-incognito window by design. Perhaps I missed something? Could someone please try out Test 3 above again?

As such, setting newTabUrl option in vimium to pages/blank.html should only work for non incognito windows. In other words, providing a blank.html with extension will not work for users working in incognito mode. Those who seek consistency throughout chrome and vimium's settings can still apply PR #1532 and set chrome's new tab setting as well as vimium's newTabUrl to some blank page.

@gdh1995
Copy link
Contributor

gdh1995 commented Mar 23, 2015

@leoj3n :
Setting the new tab page to pages/blank.html in Vimium Options and pressing t opens a new tab but does not leave focus on address bar

As I understand it, leoj3n has set Chrome's new tab url to chrome-extension://***/pages/blank.html, or use an extension to redirect new tab to such a url.
If so, here are my solution:
I find, Chrome will treat any other new tab pages as chrome://newtab. That is, Chrome must focus on its omni bar, no matter what extension the page belongs to.
Then, We have to make a jump:

  • add newtab.html to Vimium:
    <!DOCTYPE HTML><html><head><script type="text/javascript" src="newtab.js"></script></head><body></body></html>
  • add newtab.js:
    var a=localStorage.newTabUrl;chrome.tabs.create({url:a?JSON.parse(a):"/index.html"});window.close();
  • set Chrome's new tab url to chrome-extension://***/pages/newtab.html
    As a result, Ctrl+T will open a tab which opens another "normal" tab and closes itself at once, and in the new page we can see, vimium and focus aree both prepared.
  • press t will open the same url as what Ctrl+T does

@mfarrugi
Copy link

Is this the same issue as being unable to remove the vimium exclusion on chrome://newtab/*?

@smblott-github
Copy link
Collaborator Author

@mfarrugi. If you mean "Vimium does not work on chrome://newtab", then yes.

(I wrote a tiny extension (here) to get around this, if you're happy with setting your own new-tab page.)

@mfarrugi
Copy link

Ah. Unfortunately, like nikess, I would've liked it to work with an extension (SpeedDial2), even if browser focus snapped to the omnibar to start.

Thanks though!

@sunnijie
Copy link

To use vimium commands after creating a new tab with Ctrl-T, just press <tab> when Chrome focuses the omnibox.

@Chekote
Copy link

Chekote commented Jun 15, 2017

I have this problem with Chrome 59.0.3071.86. It doesn't matter what is focused. If you are on the new tab page, vimium doesn't work at all. You have to navigate to a real webpage for it to work.

@danielbaniel
Copy link

I have the same issue described by Chekote.
Version 61.0.3163.100 (Official Build) (64-bit)

@adam-stamand
Copy link

I've got the same issue. After creating a new tab, vimium ceases to do anything at all, regardless of focus. You have to navigate to a different webpage before it begins working.

Version 62.0.3202.94 (Official Build) Built on Ubuntu , running on Ubuntu 16.04 (64-bit)

@adam-stamand
Copy link

I should have searched more carefully! The issue is addressed here #2845

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

12 participants