372127 - wrong items order of nw selects

Status: RESOLVED WORKSFORME

(Core Graveyard :: XForms, defect)

Description

[alexander :surkov (:asurkov)]

Attachment: testcase

I think it's regression from 339827.

Comment 1

[aaronr]

The testcase seems backwards, Alex.  Isn't the order correct in the full select list and incorrect in the compact select?

Comment 2

[alexander :surkov (:asurkov)]

(In reply to comment #1)
> The testcase seems backwards, Alex.  Isn't the order correct in the full select
> list and incorrect in the compact select?
> 

Yes, for sure.

Comment 3

[aaronr]

I started debugging this.  Problem is in handleInsertion.  It assumes that an itemset's anoncontent has already been populated by the time that handleInsertion is called.  However, since handleInsertion is called when the XBL binding attaches for the select and itemset doesn't create its anonymous items until InitializeControls goes through the model's list of controls, there can be scenarios where handleInsertion will run before itemset's refresh().  The itemset items eventually get inserted into the select due to the mutation events, but not until all of the select's item children are added ahead of them.

Comment 4

[aaronr]

ok, I think I can fix this reasonably simply.  I chatted with Alex about this and he said that the mutation handlers are for:

1) itemset management -> as an item is added or deleted to the nodeset, the change is reflected in the select/select1
2) being able to handle dynamic insertions of itemsets, choices and items under the select/select1

The more I think about it, I think that we can handle these dynamic insertions via  DocumentChanged and ParentChanged notifications, building on the approach I plan to use for this bug.  But I think that is a lower priority and I'd like to handle that via a different bug.  Especially for item and choices.  Itemset we can probably do on this bug.  One of the issues that we'll have to overcome in this other bug is how to handle the insertions smoothly so that the user doesn't see each item being added/removed individually if an itemset rebuilds its content.

What I'd like to do for this bug is to:
1) remove the mutation handlers.  They are toooooo expensive.  Even if they do nothing but check a boolean and return they add a lot of time to our processing.
2) move the initial automatic generation of items in the native widget from being in the xbl constructor to the refresh method of the selectsnwXXX binding and only run it when requested to (i.e. after an itemset refresh or upon initialization).  This will solve the ordering problem that this bug reports.
3) From C++ be able to tell the select1 when it needs to repopulate itself.  For example, if an itemset refreshes, it will destroy and regenerate all of its anonymous items to reflect the changes that may have happened in the nodeset that it is bound to.
4) Because I plan to remove the mutation handlers, I'll have to solve the problem of a label changing its value.  The corresponding native widget item will need to get updated.

Comment 5

[aaronr]

moving to 0.9 since this scenario isn't that highly used, I don't think (mixing itemsets with items).  I'd like to fix this as part of the work for bug 372197 and that can't be done safely in time for 0.8.  Too much risk of regression.

Comment 6

[Brad Clements]

I have a use case for items and itemsets in the same select. I have a dynamic list of choices, but also a short list of the most common choices. I used items for the  common choices "at the top", then itemset for "all the rest".

Comment 7

[alexander :surkov (:asurkov)]

(In reply to comment #6)
> I have a use case for items and itemsets in the same select. I have a dynamic
> list of choices, but also a short list of the most common choices. I used items
> for the  common choices "at the top", then itemset for "all the rest".
> 

You could upload it, once someone get back to the patch will ensure it works as well.

Comment 8

[Alexey Gaidukov]

Attachment: testcase's screenshot on 0.8.7

Looks like fixed.
Aaron, can you confirm?

Comment 9

[aaronr]

I confirmed this testcase looks correct on 0.8.7 using FF 3.6.3 on windows.  Closing as worksforme.