16924 - Cannot set the focus on a non-visible input element into a scrollable tree

Status: VERIFIED FIXED

(Core :: XUL, defect, P2)

Description

[Jean-Francois Ducarroz]

It's another variation of the focus bug 15002. But this case is different. The problem occurs with message compose
addressing widget. But for simplify the problem, I will describe it with a sample xul file attached to this bug
report:

I have a tree with 4 rows defined, each of them has an input field. The defined tree height allow only 3 rows to be
visible. Each
time I press return into one of the input element, the next input field will get the focus. That works fine until the
next row isn't
visible, in this case input.focus() failed with the error -2147467262 (component does not have requested interface).

How to reproduce it:
------------------
1) Install attached files addressWidget.xul and addressWidget.js into your HD at the same location
2) open addressWidget.xul into the browser (Mozilla)
3) type something into the first input field and then press <return>
 --> second input field get the focus
4) type something into the second input field and then press <return>
 --> third input field get the focus
5) type something into the third input field and then press <return>
 --> JS exception throw, the third input field keep the focus!

Expected result: the tree should scroll up one row and give the focus to the fourth input field which is now visible

Comment 1

[Jean-Francois Ducarroz]

oops, don't need addressWidget.js, the .xul is enough...

Comment 2

[Jean-Francois Ducarroz]

Attachment: XUL file for reproducing the problem

Comment 3

[Peter Trudelle]

Have you tried this using a table instead? Would that be a viable workaround?
reassigning to saari as p2 for m11.  should this be a dogfood candidate?

Comment 4

[Jean-Francois Ducarroz]

I will try to convert the addressingWidget to use table. The main reason we use tree instead table is that we will

need to be able to specify the height of the tree in term of rows. As this feature isn't implemented yet, table should

be fine...

Comment 5

[Peter Trudelle]

mass-moving most m11 bugs to m12

Comment 6

[Jean-Francois Ducarroz]

We cannot use table as you cannot control the height of a table. All rows are
visible.

Comment 7

[saari (gone)]

I think this is due to the tree widget's lazy building of rows... you're trying
to focus a row that isn't built yet. Hyatt, does that sound right?

Comment 8

[Jean-Francois Ducarroz]

do we have a work around (else than using JS timer do delay the focus call)?

Comment 9

[David Hyatt]

Oh God.

This sucks.

This really sucks.

Do you REALLY have to use a tree widget?  You're pushing it in ways that I just
can't deal with.

Comment 10

[David Hyatt]

You could use a table instead of a tree and constrain its height either by
making a tbody with an overflow auto and a height constraint or by making a
constrained iframe and just throwing content into that.

Comment 11

[Peter Trudelle]

Mass-moving non-PDT+ bugs to M13

Comment 12

[saari (gone)]

David, I'm tossing you this bug. I guess maybe alec should get it now?

Comment 13

[David Hyatt]

Ok, I believe alec has given you the capability to scroll to a certain row.  You 
can scroll first, and then call focus.  Tossing back to you.

This is true, right alec?

Comment 14

[Jean-Francois Ducarroz]

Change it for M14. Forcing the row to scroll before setting its value may fix several bugs.

Comment 15

[Alec Flett]

yep, you can call tree.ensureContentIsVisible() (you can use any element in the
tree, and it will bring that element's row into view) and then focus it....
though I don't know exactly how this works with reflows, etc...
ensureContentIsVisible will not actually take effect until the next reflow,
which means it might not happen until after the your JS has finished
executing... 

I think hyatt can answer this one with my explanation. Hyatt, basically it
behaves just like the user dragged the thumb the exact minimum amount in order
to bring that row into view, so it does that whole smart-scrolling thing where
it destroys only the minimum number of frames to get there, and leaves a content
chain for the next reflow.

Comment 16

[Alec Flett]

btw, Jean-Francois - this is something you need to call (assuming it works of
course) from the compose window, not something the tree can do. stop by if you
need a better explanation

Comment 17

[Jean-Francois Ducarroz]

I have a fix. That also fix bug 21280

Comment 18

[Jean-Francois Ducarroz]

Fixed and checked in

Comment 19

[Jean-Francois Ducarroz]

oops, change resolution to fixed

Comment 20

[Claudius Gayle]

ducarroz, does your attached testcase work for you? For me, with the 2000013111 build on WinNT I can't even get focus to the 
first textfield much less type into it or any of the others. AM I being blocked by another new bug or is the testcase not valid?

Comment 21

[Claudius Gayle]

The testcase doesn't work and nothing I've seen from the addressing widget in mail compose makes me think this is fixed. I 
suspect another/other bugs may be blocking me but without any more information I am forced to REOPEN this bug.

Comment 22

[Jean-Francois Ducarroz]

I don't know if my test case works but the way to test it is to enter 4 
recipients by using only the enter key, the cursor should be alway into the new 
field after pressing enter.

But because of bug 26344, you cannot verify this yet.

Comment 23

[Claudius Gayle]

VERIFIED fixed with 2000020708 WinNT build