385459 - The new Page Info dialog should preserve old dialog's tabbox keyboard shortcut

Status: RESOLVED WONTFIX

(Firefox :: Page Info Window, enhancement)

Description

[(no longer active)]

Attachment: Patch to add the tab switching hotkeys

The new Page Info dialog's header (which includes a radiogroup containing radio button, each one responsible for activation of one of the tab panels of the dialog) should support the keyboard navigation mechanisms supported by the old Page Info dialog which used a tabbox.  These shortcut keys include:

 * Ctrl+Tab: move to the next tab
 * Shift+Ctrl+Tab: move to the previous tab
 * Ctrl+PgUp: move to the previous tab
 * Ctrl+PgDn: move to the next tab
 * Meta+Left: move to the previous tab (Mac OS X specific)
 * Meta+Right: move to the next tab (Mac OS X specific)
 * Home: move to the first tab (when the tab bar has the focus)
 * End: move to the last tab (when the tab bar has the focus)

This will provide better keyboard accessibility for switching tabs.  I have a patch ready which adds this behavior which I'll attach to this bug.  The patch works on the trunk.

Comment 1

[(no longer active)]

Comment on attachment 269367 [details] [diff] [review]
Patch to add the tab switching hotkeys

The code in the patch is based on the code in toolkit's tabbox widget.

Comment 2

[Florian Quèze [:florian]]

The new header is based on the header of the new addons manager. Are these shortcut keys supported in the addons manager?

Comment 3

[(no longer active)]

(In reply to comment #2)
> The new header is based on the header of the new addons manager. Are these
> shortcut keys supported in the addons manager?
> 

No, but I'm going to open a bug in the appropriate module for the Add-ons Manager (as well as the Preferences window) after I get a review for the patch attached to this bug.

For the Add-ons Manager dialog, this would be a simple enhancement.  For the Page Info dialog, this is more important IMO, because the old dialog did expose this behavior, and I don't see any reason not to support it in the new dialog.  This not being supported leads to surprises for the users used to utilize these hotkeys in the old dialog (me included).

Comment 4

[Florian Quèze [:florian]]

(In reply to comment #3)
> 
> No, but I'm going to open a bug in the appropriate module for the Add-ons
> Manager (as well as the Preferences window) after I get a review for the patch
> attached to this bug.

I thought, maybe we could do this in one place and avoid duplicating code...

Comment 5

[(no longer active)]

(In reply to comment #4)
> I thought, maybe we could do this in one place and avoid duplicating code...
> 

I see your concern.  Does the current Page Info code use any common code with the Add-ons Manager?  If I'm not mistaken, the current code is just based on the Add-ons Manager dialog, and doesn't actually share code with it.

I guess if we want to share code with the Add-ons manager, we need to place the shared code in toolkit, because there are other applications that use the Add-ons Manager dialog besides browser.  Isn't it?

Comment 6

[Florian Quèze [:florian]]

(In reply to comment #5)
> Does the current Page Info code use any common code with
> the Add-ons Manager?

They share chrome://mozapps/content/extensions/extensions.xml#viewbutton

This is called from:
browser/base/content/pageinfo/pageInfo.css

Comment 7

[(no longer active)]

(In reply to comment #6)
> They share chrome://mozapps/content/extensions/extensions.xml#viewbutton
> 
> This is called from:
> browser/base/content/pageinfo/pageInfo.css
> 

Hmmm, I didn't notice this.  But this binding is for the buttons only, we need to handle the "tabbox-like" behavior at the radiogroup level.

I looked at toolkit/contents/widgets/preferences.xml, and it's not using any special binding for the radiogroup.  Maybe it's better to first open a toolkit bug to add a binding implementing this behavior in preferences.xml, and then change the code in Page Info and Add-ons Manager to use that binding?  This way, no code is duplicated, and also all of the extension-writers who use the preferences bindings for their extension's options dialogs will get benefit without needing to change anything in their code.

Ideas?

Comment 8

[Florian Quèze [:florian]]

Moving the widget out of extensions.xml was suggested in bug 339102 comment 7.
Maybe we want to have a binding like tabbox but with the new style of tabs?

Comment 9

[(no longer active)]

(In reply to comment #8)
> Moving the widget out of extensions.xml was suggested in bug 339102 comment 7.
> Maybe we want to have a binding like tabbox but with the new style of tabs?
> 

mano: I can't seem to find the bug mentioned to move the viewButton binding to a more generic place in bug 339102 comment 7.  Did you open this bug?  Can you let me know the bug's ID?

Florian: Do you think this should be added as a new XUL widget, or something more specific?  If the latter, where do you think would be appropriate for such a binding?

Comment 10

[Asaf Romano (gone)]

I didn't open that bug IIRC... Either way I don't think this belogns to pageInfo.js.

Comment 11

[(no longer active)]

Comment on attachment 269367 [details] [diff] [review]
Patch to add the tab switching hotkeys

(In reply to comment #10)
> I didn't open that bug IIRC... Either way I don't think this belogns to
> pageInfo.js.
> 

OK.  I'll open another bug on another component (probably the toolkit) to see if we can have this in form of a binding which can be used in Page Info as well.  I'll add a dependency as soon as I open that bug.

Comment 12

[(no longer active)]

Updating to reality: I won't have the time to work on this for the foreseeable future!

Comment 13

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

I don't think we're going to invest in adding these shortcuts to these dialogs.