808420 - Alt+[tab number] in print preview switches to tab with print preview chrome

Status: NEW

(Firefox :: Tabbed Browser, defect)

Description

[Graeme McCutcheon [:graememcc]]

STR:
- Open a new window, with a URL of your choice in the first tab
- Click on new tab, load URL of your choice in this tab
- File->Print Preview in tab 2
- Focus "Page" page number input
- Alt+1

Expected:
 - Ignore the tab switch command or
 - Switch to the tab, but drop out of print preview

Actual:
Tab 1 rendered normally with print preview chrome

Comment 1

[Graeme McCutcheon [:graememcc]]

Attachment: v1

Given Ctrl+Tab doesn't work in PP, it makes sense to switch off these shortcuts to be consistent.

Adding the if into the oncommand is a little ugly, but I'm not sure it's worth adding some sort of wrapper function in browser.js instead.

I was tempted to handle this in the keypress listener in printUtils.js in toolkit, by adding Alt to the list of keys whose events we stop there, but was worried about affecting the menu bar on Windows.

We probably should disallow function keys there though - you can enter full screen and other silliness in PP. However, there's an existing bug on file somewhere about that - I'll tackle it there.

Testing print preview turns out to be a PITA.

Comment 2

[Graeme McCutcheon [:graememcc]]

Attachment: v1

Graeme, please try and hg export at the right place in your mq patch series.

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 678835 [details] [diff] [review]
v1

If the print preview UI doesn't handle tab switching, seems like we should put a "don't allow tab switches" check at a much lower level than each individual keyboard shortcut that allows it. Or, make print preview watch for TabSelect events and close itself, or something. Either of those don't seem too hard, so I'd rather do that, but it's likely I'm missing something, so let me know what you think.

Comment 4

[Graeme McCutcheon [:graememcc]]

Given Ctrl+Tab is already supressed by print preview's own keypress listener - it suppresses all Ctrl shortcuts - I think the former would be better. If we go with the second, users might wonder why they can tab switch with Alt+1 but not Ctrl+Tab. I think it's best to be consistent here.

Would it be OK to add a boolean preventTabSwitch field to tabbrowser, patching the relevant methods there (selectTabAtIndex and updateCurrentBrowser at first glance), and have the print preview listener set the field on enter/exit?

Comment 5

[Dão Gottwald [:dao]]

(In reply to Graeme McCutcheon [:graememcc] from comment #1)
> I was tempted to handle this in the keypress listener in printUtils.js in
> toolkit, by adding Alt to the list of keys whose events we stop there, but
> was worried about affecting the menu bar on Windows.

There's no menu bar in print preview mode. However, you need to make sure not to break access keys.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

(In reply to Graeme McCutcheon [:graememcc] from comment #4)
> Given Ctrl+Tab is already supressed by print preview's own keypress listener
> - it suppresses all Ctrl shortcuts - I think the former would be better. If
> we go with the second, users might wonder why they can tab switch with Alt+1
> but not Ctrl+Tab. I think it's best to be consistent here.

We could remove that suppression if we made it properly handle tab switches. But it would probably be surprising for tab switching to work since there's no indication that you're in a tab while in print preview mode, IIRC (on Mac at the moment so can't easily check).

Comment 7

[David Lawrence [:dkl]]

Changing qe-verify? to qe-verify+.