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

Support for multiple windows #17

Closed
orschiro opened this issue Feb 28, 2018 · 40 comments
Closed

Support for multiple windows #17

orschiro opened this issue Feb 28, 2018 · 40 comments
Assignees

Comments

@orschiro
Copy link
Owner

Should work with multiple windows too.

@orschiro
Copy link
Owner Author

orschiro commented Mar 4, 2018

@Bibernull I cleaned up the code in the recent commit to prepare for this issue.

Any idea how we could add support for switching between multiple windows?

@yilinjuang
Copy link
Collaborator

@orschiro i can think of two different ways of supporting multi-window.

  • users have global last tabs among all windows
  • users have independent last tabs for each window

Any ideas which is prefered?

@orschiro
Copy link
Owner Author

@frankyjuang in my opinion we should encourage people to maintain independent last tabs for each window and thus use windows to structure their work tasks. Accordingly, move a tab to the window where it belongs to if you want to switch between it.

Do you agree with me?

@yilinjuang
Copy link
Collaborator

Sounds really great for me. Will leave it until other bugs are solved.

@orschiro
Copy link
Owner Author

@frankyjuang currently testing the latest source in preparation of version 11.

However, the extension stops working after a while even in single window mode.

Can you please test too the latest commit?

@yilinjuang
Copy link
Collaborator

Really? Did you update to the HEAD? It works perfect for me after using couple of days with three windows. No error was logged. Or can you provide the logs from the background page console?
image

@orschiro
Copy link
Owner Author

Was able to capture this:

https://youtu.be/FJ0eQWhJCTY

@yilinjuang
Copy link
Collaborator

Can you set the log level to all levels (including verbose)? More debugging infos will then be displayed.
image

@orschiro
Copy link
Owner Author

@frankyjuang done. Does this produce any new insights?

https://youtu.be/-ThiBZ_09-k

@yilinjuang
Copy link
Collaborator

@orschiro I really can't tell....the error just came from nowhere.
Add this line right at the beginning of function removeTabFromWindow (Ln. 130).

console.warn(windows, windowId, tabId, currentTabId);

and see what it says when the mystery occurs.

@orschiro
Copy link
Owner Author

Hope it helps!

Screencast: https://youtu.be/EFYwx-vlv04

@yilinjuang
Copy link
Collaborator

Really weird...It seems that when you open a new tab in the background at 0:55, a onActivated event is triggered on the same page. How did you open the new tab in the background? I saw a cute image popup. Did you use some extensions that might related to this behavior?

Btw, my guess about glitch due to tab removal is wrong. The extra console.warn is totally useless 😭
This time please add it right at the beginning of function chrome.browserAction.onClicked.addListener.

console.warn(windows, currentWindowId, tab, currentTabId);

Let's see what we can do about this mystery...

@orschiro
Copy link
Owner Author

I saw a cute image popup. Did you use some extensions that might related to this behavior?

Good catch!

Will test if it conflicts with:

  1. https://chrome.google.com/webstore/detail/wunderlist-new-tab/fgikemaeelgbhjnhnnahcpkjpafaeion
  2. https://chrome.google.com/webstore/detail/open-tabs-next-to-current/gmpnnmonpnnmnhpdldahlekfofigiffh

These two I am using that my interfere here. Will let you know!

@orschiro
Copy link
Owner Author

New screencast: https://youtu.be/H4cTBfCgoD4

@yilinjuang
Copy link
Collaborator

I think I kind of know what happened. It's about the behavior of window manager. According to chrome.windows.onFocusChanged,

Fired when the currently focused window changes. Will be chrome.windows.WINDOW_ID_NONE if all chrome windows have lost focus. Note: On some Linux window managers, WINDOW_ID_NONE will always be sent immediately preceding a switch from one chrome window to another.

reference

What you encountered is that the window manager seemed to switch to a non-chrome window but never switch back...That's why you see the Object -1 Object null. -1 means id of non-chrome window. null is the currentTabId if focusing on non-chrome window. And since it is all about window manager, I'm not having the same issue on macOS.

Which window manager are you using? The default one of GNOME 3? Mutter? Perhaps we should search for similar issues?

@orschiro
Copy link
Owner Author

orschiro commented Mar 24, 2018 via email

@orschiro
Copy link
Owner Author

Ref: https://gitlab.gnome.org/GNOME/gnome-shell/issues/151

@orschiro
Copy link
Owner Author

@frankyjuang meanwhile, how about we add a new setting to context menu for people to opt in to multiple window support? Would be disabled by default. This way people on non-Linux can already test and benefit.

@yilinjuang
Copy link
Collaborator

This could be a short-term fix. But eventually I think multi-window support should be turned on by default.

@yilinjuang
Copy link
Collaborator

Any updates here?

@orschiro
Copy link
Owner Author

orschiro commented Apr 3, 2018

Unfortunately haven't heard back from the GNOME devs.

I guess next step is testing with non GNOME Linux environments and see if they are also affected.

@orschiro
Copy link
Owner Author

@frankyjuang did some tests with GNOME 3.28 but couldn't get latest commit to work. As soon as I load unpacked, it's breaking:

image

Not sure how to fix this or how to proceed.

@yilinjuang
Copy link
Collaborator

@orschiro which chrome version are you using? I cannot reproduce on latest GNOME.

@orschiro
Copy link
Owner Author

@frankyjuang tested with a brand new Chrome 66 profile and latest git pull. Loaded unpacked from src but still getting this annoying tabHistory error. Are you doing anything differently?

Error in event handler for browserAction.onClicked: TypeError: Cannot read property 'tabHistory' of undefined
    at <URL>
_generated_background_page.html:1 Error in event handler for browserAction.onClicked: TypeError: Cannot read property 'tabHistory' of undefined
    at chrome-extension://ackkikfiigagnmobecoofdoaffneiadl/background.js:65:41

@yilinjuang
Copy link
Collaborator

As far as I know, no. Same setup but no such error.

@orschiro
Copy link
Owner Author

orschiro commented May 4, 2018

Version 11 Multiple Windows has been published. Let's wait for some more user feedback! :-)

@orschiro
Copy link
Owner Author

orschiro commented May 7, 2018

@frankyjuang reinstalled version 11.1 from Chrome Webstore and it breaks: https://chrome.google.com/webstore/detail/switch-recent-tabs/odhjcgnlbagjllfbilicalpigimhdcll

A number of people report similar issues:

@orschiro
Copy link
Owner Author

orschiro commented May 7, 2018

@amorpheus
Copy link

amorpheus commented May 7, 2018

My issue is that the extension will work normally after starting Chrome, but not for long. I only work with one window. After coming back to Chrome from other programs, there's no Last Tab functionality at all. If I open a new Chrome window and close it again, I can get the extension to shortly work sometimes, but I haven't been able to pin down the exact behavior.

Running Windows 10 Enterprise (Build 1703) on a Xeon E3 1230-v3 with 16GB RAM.

@yilinjuang
Copy link
Collaborator

@amorpheus can you confirm if it only happens after switching to and back from other programs? and does it occurs right after switching back? thanks

@amorpheus
Copy link

Okay, I went through it again. Here's what I observed:

  1. completely close Chrome including task bar icon (background task)
  2. start Chrome, extension works normally
  3. switch focus to another program (I just clicked the Windows task bar)
  4. focus back to Chrome, immediately hit Ctrl-Q as I've set it, extension often works once, sometimes twice or not at all

My take is that it seems to be functional for only a short interval after switching back to a Chrome window.

@orschiro
Copy link
Owner Author

orschiro commented May 7, 2018

@amorpheus can you check if you get the error in the console shown here: #17 (comment)

@amorpheus
Copy link

amorpheus commented May 7, 2018

Just opening the developer console with F12? I'm not seeing that there. Just some "Failed to load" messages, presumably from adblock.

@orschiro
Copy link
Owner Author

orschiro commented May 7, 2018

@amorpheus do a right mouse click on the extension icon > manage extension > background page

Actually great observation! I can confirm that switching app windows breaks it after a few seconds of returning to Chrome.

@frankyjuang any ideas why this is happening and apparently only on Windows and Linux?

@amorpheus
Copy link

Alright, I see the same error as in the screenshot now.

@sxhexe
Copy link

sxhexe commented May 7, 2018

I can confirm seeing the same error as in the screenshot .

Here's my screenshot. I'm on Windows 10, Chrome Version 66.0.3359.139 (Official Build) (64-bit).

image

The behavior I see is whenever you switch to a different App (can even be another Chrome window) and switch back, Alt+Q will have an effect for once, and it always goes to the same tab (that might have been set at some point). After that Alt+Q doesn't do anything.

@eljejer
Copy link

eljejer commented May 8, 2018

Trying to debug Alt Q not working. There seems to be no error in the backgroundpage (screenshot)
capture

@orschiro
Copy link
Owner Author

orschiro commented May 8, 2018

Thanks for all your help! I have released a temporary release 12 that reverts the changes and should work stability for you until we have solved this issue. You should get the latest version in a few minutes.

@orschiro
Copy link
Owner Author

orschiro commented May 9, 2018

Here are some more debug infos from people who had troubles:

Client version: 5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.139 Safari/537.36

Client version: 5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.139 Safari/537.36

Client version: 5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.139 Safari/537.36

Client version: 5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.139 Safari/537.36

Client version: 5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.139 Safari/537.36

Client version: 5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.139 Safari/537.36

Client version: 5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.139 Safari/537.36

@orschiro
Copy link
Owner Author

User feedback.

Multiple window support should become optional, i.e. implemented with a context menu option. :-)

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

5 participants