Open Bug 1596215 Opened 9 months ago Updated 7 months ago

A tabbox (for instance, Certificate Manager) should stop "tab navigation" keyboard shortcuts from being consumed by the tabbrowser

Categories

(Toolkit :: XUL Widgets, defect, P3)

51 Branch
Desktop
Windows 10
defect

Tracking

()

Tracking Status
firefox-esr68 --- wontfix
firefox70 --- wontfix
firefox71 --- wontfix
firefox72 --- fix-optional

People

(Reporter: alice0775, Unassigned)

References

(Regression)

Details

(Keywords: regression)

Reproducible: always

Steps TO Reproduce:

  1. Open several tabs
  2. Open about:preferences#privacy
  3. Click "View Certificate..." button
    --- observe selected tab of the "Certificate Manager" sub-dialog
  4. Ctrl+PageUp to switch previous browser tab
  5. Ctrl+PageDown to switch original browser tab
    --- observe selected tab of the "Certificate Manager" sub-dialog

Actual Results:
tab of the "Certificate Manager" sub-dialog and main browser's tab are switched at the same time.

Expected Results:
If browser chrome has focus, tab of the "Certificate Manager" sub-dialog should not be switched.
If "Certificate Manager" sub-dialog has focus, tab of the "Certificate Manager" sub-dialog should be switched. And main browser tab should not be switched.

Component: XUL → Preferences
Product: Core → Firefox

Brian, you moved this to Fx::Preferences, but although this is probably the only place it happens, I expect this is an issue with event listeners for either tabbrowser or for all tabs. It looks like the tabstrip code explicitly ignores whether the event has been consumed ( https://searchfox.org/mozilla-central/rev/01d3f084379bd52eec84f56a89dceb078b93ba41/toolkit/content/widgets/tabbox.xml#127-129 before, now https://searchfox.org/mozilla-central/rev/524bed6dfbc5ae21c62632d83b7573448b29e0ac/toolkit/content/widgets/tabbox.js#135-136 ). This has been the case since at least bug 1008772.

I think this is a tabbox bug (so belongs in Toolkit :: XUL Widgets) and it should be fixed by making the handling code there call stopPropagation(). Does that make sense or am I missing something?

Flags: needinfo?(bgrinstead)

Any updates here, bgrins?

(In reply to :Gijs (he/him) from comment #2)

Brian, you moved this to Fx::Preferences, but although this is probably the only place it happens, I expect this is an issue with event listeners for either tabbrowser or for all tabs. It looks like the tabstrip code explicitly ignores whether the event has been consumed ( https://searchfox.org/mozilla-central/rev/01d3f084379bd52eec84f56a89dceb078b93ba41/toolkit/content/widgets/tabbox.xml#127-129 before, now https://searchfox.org/mozilla-central/rev/524bed6dfbc5ae21c62632d83b7573448b29e0ac/toolkit/content/widgets/tabbox.js#135-136 ). This has been the case since at least bug 1008772.

I think this is a tabbox bug (so belongs in Toolkit :: XUL Widgets) and it should be fixed by making the handling code there call stopPropagation(). Does that make sense or am I missing something?

To be clear - is the intended behavior here is that the browser changes tabs and the cert manager UI doesn't?

Because I was thinking that the cert manager UI in preferences would receive the event first so it would need to either ignore it if we want the browser to change tabs, or consume it and be in charge of stopping propagation / preventing default if we wanted only the cert manager to change tabs.

Flags: needinfo?(bgrinstead) → needinfo?(gijskruitbosch+bugs)

(In reply to Brian Grinstead [:bgrins] from comment #4)

To be clear - is the intended behavior here is that the browser changes tabs and the cert manager UI doesn't?

I think comment #0 is correct:

Expected Results:

  1. If browser chrome has focus, tab of the "Certificate Manager" sub-dialog should not be switched.
  2. If "Certificate Manager" sub-dialog has focus, tab of the "Certificate Manager" sub-dialog should be switched. And main browser tab should not be switched.

(1) is already happening. But in the steps in comment #0, we auto-focus into the dialog when it's opened, so the dialog has focus, and then I think it's up to the cert manager tab binding to consume the keypress and stop it from making its way to the browser's tabbox.

Component: Preferences → XUL Widgets
Flags: needinfo?(gijskruitbosch+bugs)
Product: Firefox → Toolkit

The priority flag is not set for this bug.
:bgrins, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(bgrinstead)

(In reply to :Gijs (he/him) from comment #5)

(In reply to Brian Grinstead [:bgrins] from comment #4)

To be clear - is the intended behavior here is that the browser changes tabs and the cert manager UI doesn't?

I think comment #0 is correct:

Expected Results:

  1. If browser chrome has focus, tab of the "Certificate Manager" sub-dialog should not be switched.
  2. If "Certificate Manager" sub-dialog has focus, tab of the "Certificate Manager" sub-dialog should be switched. And main browser tab should not be switched.

(1) is already happening. But in the steps in comment #0, we auto-focus into the dialog when it's opened, so the dialog has focus, and then I think it's up to the cert manager tab binding to consume the keypress and stop it from making its way to the browser's tabbox.

OK, thanks for the explanation. Makes sense - we should update the behavior. Probably somewhere around https://searchfox.org/mozilla-central/rev/053826b10f838f77c27507e5efecc96e34718541/toolkit/content/widgets/tabbox.js#140-159 doing stopPropagation as described in Comment 2.

Flags: needinfo?(bgrinstead)
Priority: -- → P3
Summary: Conflict shortcut key between tab of "Certificate Manager" and main browser tab → A tabbox (for instance, Certificate Manager) should stop "tab navigation" keyboard shortcuts from being consumed by the tabbrowser
You need to log in before you can comment on or make changes to this bug.