Closed Bug 402104 Opened 13 years ago Closed 13 years ago
Places Organizer window misses accesskeys
Select the Bookmarks > Organize Bookmarks... menu, in the displayed window, it misses several accesskeys, especially in the bottom panel. It also misses accesskeys in the Toolbar for "Organize" and "Import and Backup" buttons. I will provide a patch till the end of this week
Comment on attachment 287114 [details] [diff] [review] Patch adding accesskeys to Places Adding requestee
Attachment #287114 - Flags: review? → review?(mano)
DUPLICATE of bug 400703 ?
Aaron, is this bug a duplicate of bug 400703 ? If so, does the attached patch here adresses the concerns of bug 400703 ? Thanks
Not at dupe of bug 400703 -- the missing accesskeys is just one part of the accessibility problems in bug 400703. The buttons should be in the tab order, and even when the accesskeys are there (it is for one of the items), they don't seem to work.
After talking with Aaron on IRC, this one should fix this bug and bug 400703.
Attachment #288229 - Flags: review? → review?(mconnor)
>this one should fix this bug and bug 400703. When you say "should", do you mean you tested it and it works?
No, I didn't test yesterday :-\ . The toolbar buttons were tabbable but pressing entry didn't do anything and the accesskeys didn't work. I made some tests inserting a hbox for this three buttons and changing the toolbar buttons for simple buttons. This changes the UI but works. Buttons are tabbable and the accesskeys work. I will attach a screenshot of the Places Organizer with these changes
The screenshot to see the UI changes with buttons instead of toolbar buttons
Comment on attachment 288346 [details] [diff] [review] Patch changing toolbarbuttons for buttons with an hbox Hi Cedric, Can you please generate your patch using -u8p, per the instructions here: http://developer.mozilla.org/en/docs/Creating_a_patch Thanks!
Sorry, I wasn't aware of this... Here's the new patch with -u8p parameters
Attachment #288346 - Attachment is obsolete: true
Comment on attachment 288229 [details] [diff] [review] Patch 2 addressing also concerns of bug 400703 Removing review request as this patch doesn't fix all the concerns. Waiting for comments on the last attached patch
Attachment #288352 - Flags: review?(mconnor) → review?(mano)
I report here wonderings I spoke about in #granparadiso : what's the status for bug 402104 ? I know it has only P2 priority, but I just wanted to know if it is the good way to fix it, though imho, I think either the original code or my patch broke the user experience, as this window, regarding the tool bar/Menu bar , has no equivalent in Firefox UI. I'm about to post a comment about it in the bug [...] My English is weak : these are supposed to be toolbar buttons (originally), but toolbar buttons are supposed to be shown as icons only, text only, or both : so they don't have accesskeys all over the Firefox UI. In this case, they have to drop down menus, so either toolbar buttons or simple button are not the appropriate way for this. Menu items are Hope this was clearer... Well, in short : should have I to propose a patch with menu items, or is this window going to be redesigned?
Flags: blocking-firefox3? → blocking-firefox3+
Priority: P2 → P3
Comment on attachment 288352 [details] [diff] [review] Patch with -u8p parameters as requested This no longer applies, sorry. Also: 1. Please don't add accesskeys in menu in which find-as-you-type needs to work (folderMenuList) 2. Don't add accesskeys to disables controls (liveTitlesSeparator)
Attachment #288352 - Flags: review?(mano) → review-
This patch addresses comment #14. I removed class="tabbable" as buttons are tabbable by default. I'm still concerned by comment #12 : We ended up using simple buttons as toolbarbuttons don't support accesskey. Now, these buttons are accessible, but one needs to make two steps to access the dropdown menus. For instance, for "Organize", one needs to make ALT+O and Enter, and then, to access the dropdown menu items, the only way is to use the down arrow, because pressing ALT+whatever will close the dropdown menu and put the focus on the button again. imho, we should drop using the fancy toolbarbuttons or buttons with icons in this window and use regular menu items to get this accessible the way it does, unless there's a turnaround I'm not aware of or skilled to implement.
It would be better to just fix the accesskeys here, and leave the toolbar issues to a follow up. Please also make sure that the accesskeys used in the details pane also work (and don't break!) the panel in the main browser window.
(In reply to comment #16) > It would be better to just fix the accesskeys here, and leave the toolbar > issues to a follow up. Do you mean I should let the current "Organize", "Views" and "Import and Backup" items as they currently are and just fix the accesskeys in the details pane? > Please also make sure that the accesskeys used in the > details pane also work They were working at the time when I compiled this patch > (and don't break!) the panel in the main browser window. I'm not sure to understand this part : did you notice something broken with the last patch? I didn't notice.
I didn't test the patch myself yet, I'm just pointing out that these accesskeys ought not to conflict with global accesskeys in the browser window (e.g. Alt+T may conflict with the Tools menu). And yes, let's go with a follow up for the toolbarbuttons->buttons issue.
- All accesskeys work fine - Switching back and forth between the places window and the main browser window, ALT+T, for instance, behaves as expected in both windows - Find as you type feature also works as long as the focus is not in one of the fields/buttons of the details pane. Pressing ESC frees the focus, and the fayt feature works again, which is the expected behaviour I think. I will open the followup bug after this one will be closed.
So, alt+t goes the tag field when the popup is open but to the Tools menu otherwise?
Yes, it does, even if you keep the Places window open and if you switch to the main browser window, ALT+T opens the Tools menu.
Not that: 1. Open main browser one 2. click twice on the star icon to open the popup while the popup is open, alt+t should select the tags field, not open the tools menu.
Ah, ok. I misunderstood your point. I was focussing on the Places window, not the popup window from the star icon. I tested : - double clicking on the star opens the popup with the text in the "Name" field selected. - then, ALT+T put the focus in the Tags field and do not pop down the Tools menu; and one can tab cycle in the popup window.
one alt+tab, you mean? and, once you close the popup, alt+t works again for opening the tools menu?
I think we should rather allow setting different accesskeys for "More" and "Less" (and set them both to "e" in en-US).
(In reply to comment #24) > one alt+tab, you mean? I meant, once the popup is open, you can use tab to access any tabbable field/button inside the popup window. Pressing ESC close the popup. > and, once you close the popup, alt+t works again for opening the tools menu? Yes, it does.
(In reply to comment #25) > I think we should rather allow setting different accesskeys for "More" and > "Less" (and set them both to "e" in en-US). I ended up with that code to add 2 accesskeys for Less/More button. This doesn't work and I can't figure out why. To test it, I used "s" for &detailsPane.less.accesskey; but I only got the value of &detailsPane.more.accesskey;
The *property* name is accessKey, not accesskey.
Thanks Asaf. It works fine now.
Comment on attachment 297045 [details] [diff] [review] Patch with 2 accesskeys for More/less button r=mano, thanks.
Attachment #297045 - Flags: review?(mano) → review+
mozilla/browser/components/places/content/editBookmarkOverlay.xul 1.10 mozilla/browser/components/places/content/places.js 1.121 mozilla/browser/components/places/content/places.xul 1.102 mozilla/browser/locales/en-US/chrome/browser/places/editBookmarkOverlay.dtd 1.6 mozilla/browser/locales/en-US/chrome/browser/places/places.dtd 1.40
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Verified. Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2008011414 Minefield/3.0b3pre
Status: RESOLVED → VERIFIED
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h". In Thunderbird 3.0b, you do that as follows: Tools | Message Filters Make sure the correct account is selected. Click "New" Conditions: Body contains places-to-b-and-h Change the action to "Delete Message". Select "Manually Run" from the dropdown at the top. Click OK. Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter. Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.