64458 - javascript strict warnings in treeBindings.xml

Status: VERIFIED FIXED

(Core :: XUL, defect)

Description

[Blake Ross]

We should select the first item in a tree if focus is set to it, and there are 
no other selected items.  We currently do this anyways in most places, so it 
might as well be the behavior of the tree.

Comment 1

[Blake Ross]

Attachment: [patch] select first item, fix some warnings

Comment 2

[Blake Ross]

bryner/jrgm/hyatt - r/sr?

Comment 3

[John Morrison]

Hmm. The fix seems reasonable code-wise, but I'm not sure it's the right UI.

Two examples: 

(1) if a tree doesn't have focus, and the first action to cause focus to occur 
is to click on a twisty in the middle of the list, then you wind up having the 
folder expand _and_ the first item in the list selected. Looks odd (you can see 
this in the Customize Sidebar dialog). 

(2) if a tree doesn't have focus, and the first action is to click on the 
scrollbar (for a long tree), the list scrolls and the first item is selected 
(as it possibly passes out of view). (Try this in 'View Stored Cookies').

Comments?

Comment 4

[Blake Ross]

Those are both very good points.  Adding a check to make sure event.target is 
|this| seems to fix both of them.  Is that sufficient, or are there other cases 
I'm not thinking of?  Will that check cause different problems?

Comment 5

[Blake Ross]

ack, nevermind.  That check doesn't fix the scrollbar case.  Any ideas on how 
to prevent that?

I still maintain that this is the appropriate UI for this, although I 
admittedly was doing this only for the case where focus was set manually, 
instead of by the user (I can't think of a single case where you'd want to just 
focus a tree that didn't have selected items, without selecting an item.  
Selection is really the only visible indication of focus in a tree).

If the consensus against this is that it's not a good idea, then that's fine, 
but this seems like another case where we'll be doing the same thing 
everywhere, which is why I wanted this as base behavior.

Comment 6

[John Morrison]

I was just noting two places where auto-selecting was inconsistent with the 
user's action (i.e., the UI gesture was not to select something, but it 
happened automagically anyways; not a disaster, but not expected). 

I'm not sure that you always want to have an item selected whenever a tree gets
initial focus. Sometimes a tree is presented as "the possible choices" and 
pre-selecting an item may not be what a UI developer wants. (What do other 
toolkits do w.r.t. auto-selection of a tree). 

But that's just an opinion, one not fully contemplated, and not one I am really 
married to (not even engaged).

(nit: focus is indicated by focus rings in some trees, so selection is not the 
only feedback path).

Comment 7

[Blake Ross]

Yeah, I'm starting to agree that selecting on focus is giving more than was 
asked for.  How about r='ing the warning fixes and calling it a day?

Comment 8

[John Morrison]

Sure, r=jrgm for the warning fixes. 

(But I wouldn't drop the auto-select question entirely; maybe it is the way to 
go. Perhaps bryner and hyatt have thoughts (or mpt or german from the UE 
perspective)).

Comment 9

[Blake Ross]

Okay thanks, I'll speak with Matthew next time he's around irc.

Hyatt, can you sr just the warning fixes?

Comment 10

[David Hyatt]

I do not want the tree auto-selecting the first item on focus.  Too hairy.

sr=hyatt on just the warning fixes.

Comment 11

[Blake Ross]

Fix checked in.

Comment 12

[Henrik Gemal]

verified with build 20010126 on win2k

Comment 13

[Henrik Gemal]

sorry for the spam.... just changing the subject so that the bugs can be sorted 
correctly

Comment 14

[Jesse Ruderman]

Comment 0 is bug 154034, "When tabbing to listbox or tree, sometimes nothing
appears focused until you arrow down".