334015 - repeat index() doesn't reset to 0 when last repeat item is deleted

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Steve Speicher]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060411 Firefox/3.0a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060411 Firefox/3.0a1

1.0 2ed test suite case 9.3.6.a shows a index() == 1 when
all repeat items have been removed (when it should be == 0)

See related bug 282828

Reproducible: Always

Comment 1

[Steve Speicher]

Attachment: testcase

Comment 2

[aaronr]

An output that uses @value="index('repeater')" currently isn't guarenteed to have the correct value.

Looks like the problem is that output evaluates the xpath during ::Bind (though not displayed until ::Refresh) and repeat doesn't set index until its ::Refresh.  But even if output evaluated it at refresh, it still might not be right, depending if its refresh was before repeats or not.

Seems like something for the xpath dependency stuff.  Otherwise a hack would be that any control who asks for index() should get registered with the repeat somehow?  Yuck.  I don't think that would be easy to keep that hack on the XForms side of the fence w/o doing string parsing.  And it isn't just controls, also xf:bind.

I don't know if this is related to the other index bugs.  Didn't seem like it when I read the other abstracts.  For what it is worth, the repeat index seems to be right.

Comment 3

[aaronr]

Attachment: testcase 2

This testcase also shows the problem.  Notice that adding items seems ok, but then deleting items will always have index showing as 1 off.  Again, the index is right on the repeat by the time its refresh method returns.  But since the output asked for the index before repeat had changed it, the value the user sees is wrong.

Comment 4

[Allan Beaufour]

(In reply to comment #2)
> Seems like something for the xpath dependency stuff.  Otherwise a hack would be
> that any control who asks for index() should get registered with the repeat
> somehow?  Yuck.  I don't think that would be easy to keep that hack on the
> XForms side of the fence w/o doing string parsing.

It is yuck, but it is unavoidable, and we have it already. It is not handled by the MDG, because the index is this "magic thing" hanging around without being bound to the instance data. The same goes for switch state. There are probably amplications, but they should possibly have been done like instance('internal')/repeat_index/<id> and instance('internal')/swith_state/<id>

> And it isn't just controls, also xf:bind.

Bug 292333.

The problem is probably that the IndexHasChanged() code is not run when index reached zero. /me wonders it this is fixed by bug 331452.

Comment 5

[Allan Beaufour]

There seems to be generic index problems. Probably regressed from bug 331452.

Comment 6

[Allan Beaufour]

Attachment: setindex on empty nodeset testcase

Related problem showing up, when we actually manage to get index to be 0.

Comment 7

[Allan Beaufour]

Attachment: Patch

Also fixes the setindex problem (testcase in attachment 223013 [details]).

Comment 8

[Allan Beaufour]

Comment on attachment 223014 [details] [diff] [review]
Patch

I think I have a better patch coming up, which also takes care of bug 318779 too.

Comment 9

[Allan Beaufour]

Attachment: Patch

Problems were that I forgot to handle the empty nodeset case, and that I forgot to inform about index changes on all cases.

(the main problem is that the code is awful, but I do not have time to rewrite it, as it should be... :( )

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 223027 [details] [diff] [review]
Patch

r=me, I think :)

Comment 11

[Allan Beaufour]

Fixed on trunk