Fix #1507 Open new tab in incognito mode #1508
Conversation
- Change openUrlInNewTab to pass selected tab.windowId - Change createTab to use openUrlInNewTab
|
LGTM, but the comments add noise; it would be better without. Thanks @wadkar. |
|
BTW the changes are compiled and tested on my local machine. $ cake test
Running unit tests...
Pass (109/109)
Running DOM tests...
Pass (32/32) |
|
I thought comments would explain why the changes, and also refer to upstream bug. Shall I remove and push another commit to this PR? |
|
Yes please! You can add the details from the comments in the commit message, so the information is available if anybody is interested as to why the code is how it is, but doesn't add noise to the code itself. |
- Change openUrlInNewTab to pass tab.windowId Why? Work around for upstream bug #308171 - Change createTab to use openUrlInNewTab Why? Fix issue philc#1507 to open new tab in current window (Note: This commit removes noise from the code and explains the changes)
|
@mrmr1993 Let me know if this looks good. And thanks for the quick response! |
|
Looks great, thanks! |
|
My pleasure! |
|
Thanks for this, @wadkar. It looks like you might have lost the |
|
@smblott-github yep, just checked, |
|
It's because you dropped calling |
- This fix enables `2t` to open two new tabs, even in incognito. Include callback in the call chain so that numbered commands can work.
|
OK, cool. How about now? |
|
Thanks, @wadkar. Merged. |
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/
This PR is a fix for issue #1507