SetIntrinsicState is currently being initialized during nsXFormsControlStub::OnCreated. This is too early. SetIntrinsicState won't do anything until the element is in a document. It works most of the time because we'll set the intrinsic state during refresh of controls under the model's list. But this won't work for elements (like group) that are not be bound to anything.

In this testcase, focus isn't passed from a group to its first child because the GetRelevantState() call in nsXFormsGroupElement::TryFocus is returning that the group isn't relevant. This is because we only call SetIntrinsicState on a xf:group one time, during ::OnCreated.

I fixed this by moving the initialization of the intrinsic states from ::OnCreated to ::DocumentChanged.

fails 9.1.1.c in the testsuite

Comment on attachment 216472 [details] [diff] [review] proposed fix removing review requests. Doron mentioned that I should change TryFocusChildControl while I'm in a focus bug to test for child nodes being element nodes before QI'ing to be more efficient.

(In reply to comment #4) > (From update of attachment 216472 [details] [diff] [review] [edit]) > removing review requests. Doron mentioned that I should change > TryFocusChildControl while I'm in a focus bug to test for child nodes being > element nodes before QI'ing to be more efficient. > I believe QIs should be avoided if possible :)

(In reply to comment #5) > (In reply to comment #4) > > (From update of attachment 216472 [details] [diff] [review] [edit] [edit]) > > removing review requests. Doron mentioned that I should change > > TryFocusChildControl while I'm in a focus bug to test for child nodes being > > element nodes before QI'ing to be more efficient. > > > > I believe QIs should be avoided if possible :) > Found a couple of other places that do the same thing (run a child list QI'ing w/o checking type first). I'll fix them all in a seperate bug. Re-asking for review for this patch.

Comment on attachment 216472 [details] [diff] [review] proposed fix > nsXFormsControlStubBase::DocumentChanged(nsIDOMDocument *aNewDocument) > { > // We need to re-evaluate our instance data binding when our document > // changes, since our context can change > if (aNewDocument) { >+ // The intrinsic state needs to be initialized here since >+ // SetIntrinsicState will do nothing until the element lives in a document. >+ ResetProperties(); >+ > Bind(); > Refresh(); > } > > return NS_OK; > } I agree, something's not right, but doesn't the handling then belong inside Bind()? (Hmmm, actually it belongs inside Refresh(), as the changes to MIP states should not be changes before refresh, but that's a bigger issues)

(In reply to comment #7) > (From update of attachment 216472 [details] [diff] [review] [edit]) > > nsXFormsControlStubBase::DocumentChanged(nsIDOMDocument *aNewDocument) > > { > > // We need to re-evaluate our instance data binding when our document > > // changes, since our context can change > > if (aNewDocument) { > >+ // The intrinsic state needs to be initialized here since > >+ // SetIntrinsicState will do nothing until the element lives in a document. > >+ ResetProperties(); > >+ > > Bind(); > > Refresh(); > > } > > > > return NS_OK; > > } > > I agree, something's not right, but doesn't the handling then belong inside > Bind()? > > (Hmmm, actually it belongs inside Refresh(), as the changes to MIP states > should not be changes before refresh, but that's a bigger issues) > I was just looking for a place that we can initialize a control to 'enabled' just once. Bind and Refresh both get called twice even if the control doesn't have a SNB attribute.

Comment on attachment 216472 [details] [diff] [review] proposed fix (In reply to comment #8) > (In reply to comment #7) > > I agree, something's not right, but doesn't the handling then belong inside > > Bind()? > > > > (Hmmm, actually it belongs inside Refresh(), as the changes to MIP states > > should not be changes before refresh, but that's a bigger issues) > > I was just looking for a place that we can initialize a control to 'enabled' > just once. Bind and Refresh both get called twice even if the control doesn't > have a SNB attribute. I still do not think it is the correct place, but the whole SetIntrinsicState() business is somewhat defect in the first place, so ok, if this fixes your use case. r=me

This is the patch that I checked in. Updated to trunk.

doh!