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

Better behavior around tab switching via Ctrl-Tab shortcut ("Missing" legacy behavior) #1531

Closed
Sinewyk opened this Issue Nov 14, 2017 · 10 comments

Comments

Projects
None yet
4 participants
@Sinewyk

Sinewyk commented Nov 14, 2017

Short description

Navigating between tabs is less intuitive than legacy, navigating to previous tab by keyboard will expand previous tree and go to last tab of that tree (just not taking into account TST, but just regular tabs) instead of staying on the same level and going to previous tab AND THEN if we keep Ctrl pressed extending that tab (and collapsing others).

Steps to reproduce

  1. Start Firefox with clean profile.
  2. Install TST.
  3. Create this structure with tabs
A (collapsed)
\- A1
\- A2
B (collapsed)
\- B1
\- B2
C (selected)
  1. press Ctrl + Shift + Tab (or Ctrl + Page Up), the previous tab shortcut

Legacy behavior

It would go from C to B (and not expand B until pressing Ctrl for a short amount of time, a quick press would leave B collapsed)

Current behavior

It goes from C to B2 (and does not expand B by keeping Control pressed, and does not collapse C if B expands).

Environment

  • Platform (OS): Windows 10 Pro (1703, build 15063.674)
  • Version of Firefox: 57.0 (64 bits)
  • Version (or revision) of Tree Style Tab: 2.2.3

@piroor piroor changed the title from "Missing" legacy behavior to Better behavior around tab switching via Ctrl-Tab shortcut ("Missing" legacy behavior) Nov 15, 2017

@piroor

This comment has been minimized.

Show comment
Hide comment
@piroor

piroor Nov 15, 2017

Owner

I think this can be implemented partially.

  • Ctrl key is pressed
  • a collapsed tab is focused

On this situation TST should move focus to the topmost visible ancestor tab instead of collapsed children.

Owner

piroor commented Nov 15, 2017

I think this can be implemented partially.

  • Ctrl key is pressed
  • a collapsed tab is focused

On this situation TST should move focus to the topmost visible ancestor tab instead of collapsed children.

@piroor

This comment has been minimized.

Show comment
Hide comment
@piroor

piroor Nov 15, 2017

Owner

I've implemented this by 0f74d53, but there are some problems from limitations of WebExtensions APIs:

  • TST cannot detect keydown/keyop of the ctrl key on special pages (about:addons, https://addons.mozilla.org/, etc.)
    • TST can misunderstand like "now the user is not trying to switch tab focus by Ctrl-Tab/Ctrl-Shift-Tab, so I expand this newly focused collapsed tree" when you start to switching on such a special page tab.
    • TST can misunderstand like "now the user is trying to switch tab focus by Ctrl-Tab/Ctrl-Shift-Tab, so I don't expand this newly focused collapsed tree" when you focus to any tab after you released the ctrl key on any special page tab.
  • TST cannot detect keydown/keyop of the ctrl key on Firefox's UI (location bar, web search box, find bar, etc.)
    • TST can misunderstand like "now the user is not trying to switch tab focus by Ctrl-Tab/Ctrl-Shift-Tab, so I expand this newly focused collapsed tree" when you are hitting Ctrl-Tab/Ctrl-Shift-Tab on the location bar or somewhere.
Owner

piroor commented Nov 15, 2017

I've implemented this by 0f74d53, but there are some problems from limitations of WebExtensions APIs:

  • TST cannot detect keydown/keyop of the ctrl key on special pages (about:addons, https://addons.mozilla.org/, etc.)
    • TST can misunderstand like "now the user is not trying to switch tab focus by Ctrl-Tab/Ctrl-Shift-Tab, so I expand this newly focused collapsed tree" when you start to switching on such a special page tab.
    • TST can misunderstand like "now the user is trying to switch tab focus by Ctrl-Tab/Ctrl-Shift-Tab, so I don't expand this newly focused collapsed tree" when you focus to any tab after you released the ctrl key on any special page tab.
  • TST cannot detect keydown/keyop of the ctrl key on Firefox's UI (location bar, web search box, find bar, etc.)
    • TST can misunderstand like "now the user is not trying to switch tab focus by Ctrl-Tab/Ctrl-Shift-Tab, so I expand this newly focused collapsed tree" when you are hitting Ctrl-Tab/Ctrl-Shift-Tab on the location bar or somewhere.
@Sinewyk

This comment has been minimized.

Show comment
Hide comment
@Sinewyk

Sinewyk Nov 15, 2017

If I can have previous behavior except on special pages, I will gratefully accept. I can remember to not linger on those pages and always close them when I'm finished, 👍

Sinewyk commented Nov 15, 2017

If I can have previous behavior except on special pages, I will gratefully accept. I can remember to not linger on those pages and always close them when I'm finished, 👍

@SXZ1

This comment has been minimized.

Show comment
Hide comment
@SXZ1

SXZ1 Nov 15, 2017

I personally would like to be able to turn off this feature and all the related code including script injection for two reasons: flickering and possible performance/responsiveness degradation.

SXZ1 commented Nov 15, 2017

I personally would like to be able to turn off this feature and all the related code including script injection for two reasons: flickering and possible performance/responsiveness degradation.

@Sinewyk

This comment has been minimized.

Show comment
Hide comment
@Sinewyk

Sinewyk Nov 15, 2017

I suppose once the dust settles everything will be configurable. Personally I just deactivated the animation and everything is smooth.

Sinewyk commented Nov 15, 2017

I suppose once the dust settles everything will be configurable. Personally I just deactivated the animation and everything is smooth.

@piroor

This comment has been minimized.

Show comment
Hide comment
@piroor

piroor Nov 16, 2017

Owner

@SXZ1 I agree that global script injection seems bad, but dynamic injection based on configuration seems to require more resources. Because scripts injected via browser.tabs.executeScript() dynamically are executed only once, we need to listen every page load and reinject script manually. I think that is more worse...

Owner

piroor commented Nov 16, 2017

@SXZ1 I agree that global script injection seems bad, but dynamic injection based on configuration seems to require more resources. Because scripts injected via browser.tabs.executeScript() dynamically are executed only once, we need to listen every page load and reinject script manually. I think that is more worse...

@SXZ1

This comment has been minimized.

Show comment
Hide comment
@SXZ1

SXZ1 Nov 16, 2017

@piroor no problem, I am not a programmer so TST is a black box for me. Thank you very much for the explanation, TST is awesome!

SXZ1 commented Nov 16, 2017

@piroor no problem, I am not a programmer so TST is a black box for me. Thank you very much for the explanation, TST is awesome!

@protasovams

This comment has been minimized.

Show comment
Hide comment
@protasovams

protasovams Dec 1, 2017

@piroor It seems that adding script handle-accel-key.js is the cause that suddenly all the onKeyDown, onKeyUp events are logged in console. Is there a way to cancel this?

protasovams commented Dec 1, 2017

@piroor It seems that adding script handle-accel-key.js is the cause that suddenly all the onKeyDown, onKeyUp events are logged in console. Is there a way to cancel this?

piroor added a commit that referenced this issue Dec 1, 2017

@piroor

This comment has been minimized.

Show comment
Hide comment
@piroor

piroor Dec 1, 2017

Owner

Sorry I forgot to deactivate logging. bb460eb should fix it.

Owner

piroor commented Dec 1, 2017

Sorry I forgot to deactivate logging. bb460eb should fix it.

@Sinewyk

This comment has been minimized.

Show comment
Hide comment
@Sinewyk

Sinewyk Dec 28, 2017

AFAIK it's done =).

Thanks a lot.

Sinewyk commented Dec 28, 2017

AFAIK it's done =).

Thanks a lot.

@Sinewyk Sinewyk closed this Dec 28, 2017

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