After performing an operation to the selected richlistitem, it is no longer focused

RESOLVED FIXED in mozilla1.9

Status

()

Toolkit
XUL Widgets
P2
normal
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: rstrong, Assigned: dao)

Tracking

({access, regression})

Trunk
mozilla1.9
x86
All
access, regression
Points:
---
Bug Flags:
blocking1.9 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

This only affects trunk.

Steps to reproduce:
1) Select an extension in the extensions view.
2) Use arrow up or down to verify adjacent extensions can be selected by using these keys
3) Click enable or disable for the selected extension
4) Use arrow up or down to select an adjacent extension

Actual results:
entire list is scrolled if a scroll bar is displayed

Expected results:
adjacent extension is selected
Flags: blocking-firefox3?

Comment 1

10 years ago
Regression from bug 377686? If so, the question is whether to add keyboard navigation handlers for the bubble phase to richlistbox.xml in general or just to the EM binding in particular.

Comment 2

10 years ago
Created attachment 269389 [details] [diff] [review]
react to bubbling navigation key events

The same issue also exists with the Downloads Manager, so other implementors might expect this behavior as well. This patch restores the previous behavior with the slight difference that focus is returned to the richlistbox on navigation so that it doesn't get lost unexpectedly.

Note that hitting [Left] or [Right] still allows the focus to leave the richlistbox, but that's another bug if not expected behavior.
Attachment #269389 - Flags: review?(enndeakin)

Comment 3

10 years ago
Why would we want to fire two key events. Capturing listeners would think that the key was pressed twice, no?

Comment 4

10 years ago
Created attachment 269508 [details] [diff] [review]
react to bubbling navigation key events (take 2)

(In reply to comment #3)
> Why would we want to fire two key events.

The patch was missing an event.stopPropagation(); so that we'd just have replaced one event with another for best code-reuse - but during the capture phase there'd indeed have been two events instead of one.

So we can either just copy the handlers from listbox.xml, adding

  if (event.originalTarget != this) { this.focus(); ... }

to each of them, or use one common handler for all cases as this patch proposes.
Assignee: nobody → zeniko
Attachment #269389 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #269508 - Flags: review?(enndeakin)
Attachment #269389 - Flags: review?(enndeakin)

Comment 5

10 years ago
I'll need to think about this more. Maybe reverting the change to combine  the handlers between the two bindings. Maybe another Neil has some thoughts.

Comment 6

10 years ago
Well, listboxes have the same issue (I used the mail filter dialog ... create a few rows so that a scrollbar appears, then tab to a central + button and hit down and up but be careful not to hit the same key twice otherwise focus goes elsewhere). At the very least I suggest button.xml calls event.preventDefault() when advancing/rewinding focus. Once that is done it should be safe to remove phase="target" from listbox.xml and check event.getPreventDefault() instead. Note that this doesn't actually allow an adjacent extension to be selected.

Updated

10 years ago
Flags: blocking-firefox3? → blocking-firefox3+

Updated

10 years ago
Target Milestone: --- → Firefox 3 M8

Comment 7

10 years ago
Comment on attachment 269508 [details] [diff] [review]
react to bubbling navigation key events (take 2)

Removing review request as the other Neil's idea seems possible.
Attachment #269508 - Flags: review?(enndeakin)

Updated

10 years ago
Assignee: zeniko → nobody
Status: ASSIGNED → NEW

Updated

10 years ago
Target Milestone: Firefox 3 M8 → Firefox 3 M9

Updated

10 years ago
Target Milestone: Firefox 3 M9 → Firefox 3 M10

Comment 8

10 years ago
I can add that the focus is lost also when clicking on uninstall or options buttons.
This might be out of scope of this bug ? but when switching between e.g. extensions and themes tab the last selected item should get focus as it was the case with FF 2.0, currently it is selected but without focus.
Also surprisingly after having selected one item under plugins tab and relaunching EM - no item is selected so focus-last-selected-item feature does not work here, got ideas why ?
OS: Windows Vista → All
Priority: -- → P1

Updated

10 years ago
Target Milestone: Firefox 3 M10 → Firefox 3 M11

Updated

10 years ago
Priority: P1 → P3
Duplicate of this bug: 406739
We also hit this with the download manager when searching, and the external protocol handling dialog hits it all the time (at least on the mac for both of these).

I think we should probably up the priority on this?
Shawn, are there bugs for this for the download manager and the protocol handling dialog? Also, there should probably be a widget bug to cover all the consumers of the richlistbox. The last time I worked around these types of issues in extensions.xml late in the game for Firefox 2.0 which was not ideal and it would be a 'good thing' to fix this in richlistbox.xml this time around.
No - I was talking Dave about this and figured I'd comment to indicate that there is probably a larger problem with the widget and not the consumers (maybe move the bug to Toolkit:Widgets?)
Moving over to Toolkit -> XUL Widgets per comment #10 and comment #12
Component: Extension/Theme Manager → XUL Widgets
Flags: blocking-firefox3+
Product: Firefox → Toolkit
QA Contact: extension.manager → xul.widgets
Summary: After performing an operation to an add-on the selected item is no longer focused → After performing an operation to the selected item is no longer focused
Target Milestone: Firefox 3 beta3 → ---
This was already blocking Firefox 3.0 so requesting blocking 1.9
Flags: blocking1.9?

Updated

9 years ago
Duplicate of this bug: 416392

Updated

9 years ago
Flags: blocking1.9? → blocking1.9+

Updated

9 years ago
Summary: After performing an operation to the selected item is no longer focused → After performing an operation to the selected richlistitem, it is no longer focused
Flags: tracking1.9+ → blocking1.9+
Keywords: access
Priority: P3 → P2
(Assignee)

Comment 16

9 years ago
Created attachment 310085 [details] [diff] [review]
patch as per comment 6
Attachment #269508 - Attachment is obsolete: true
Attachment #310085 - Flags: review?(neil)
(Assignee)

Comment 17

9 years ago
Created attachment 310088 [details] [diff] [review]
patch as per comment 6 (typo fixed)
Attachment #310085 - Attachment is obsolete: true
Attachment #310088 - Flags: review?(neil)
Attachment #310085 - Flags: review?(neil)
Assigning to Dao based on the fact he's actually doing the work.  Thanks for the patch, Dao.
Assignee: nobody → dao
Comment on attachment 310088 [details] [diff] [review]
patch as per comment 6 (typo fixed)

r=me on the button.xml change which is good to stop the unexpected listbox scrolling but the rest of the widget set isn't ready for the listbox.xml change.
Attachment #310088 - Flags: review?(neil) → review+
(Assignee)

Comment 20

9 years ago
Do you know of specific widgets that wouldn't be ready? I didn't find any in toolkit/content/widgets/.
The one I noticed was that a menulist handles up/down but doesn't seem to call preventDefault; I think textboxes (well, textareas) might call preventDefault but that will be in the system group so you will need group="system" too.
(Assignee)

Comment 22

9 years ago
Created attachment 311128 [details] [diff] [review]
also fix menulists and textboxes

yeah, group=system makes it work for multiline textboxes, although I don't know what it's doing exactly. Doesn't seem to be documented.
Attachment #310088 - Attachment is obsolete: true
Attachment #311128 - Flags: review?(neil)
Comment on attachment 311128 [details] [diff] [review]
also fix menulists and textboxes

OK, this works with all of the (rich|)listboxes I could think of.
Attachment #311128 - Flags: review?(neil) → review+
Keywords: checkin-needed
Checking in toolkit/content/widgets/button.xml;
/cvsroot/mozilla/toolkit/content/widgets/button.xml,v  <--  button.xml
new revision: 1.22; previous revision: 1.21
done
Checking in toolkit/content/widgets/listbox.xml;
/cvsroot/mozilla/toolkit/content/widgets/listbox.xml,v  <--  listbox.xml
new revision: 1.35; previous revision: 1.34
done
Checking in toolkit/content/widgets/menulist.xml;
/cvsroot/mozilla/toolkit/content/widgets/menulist.xml,v  <--  menulist.xml
new revision: 1.40; previous revision: 1.39
done
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9beta5
Target Milestone: mozilla1.9beta5 → mozilla1.9
You need to log in before you can comment on or make changes to this bug.