Last Comment Bug 334015 - repeat index() doesn't reset to 0 when last repeat item is deleted
: repeat index() doesn't reset to 0 when last repeat item is deleted
Status: RESOLVED FIXED
: fixed1.8.0.5, fixed1.8.1
Product: Core Graveyard
Classification: Graveyard
Component: XForms (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Allan Beaufour
: Stephen Pride
:
Mentors:
http://www.w3.org/MarkUp/Forms/Test/X...
Depends on:
Blocks: 318779 322255 331209
  Show dependency treegraph
 
Reported: 2006-04-14 10:12 PDT by Steve Speicher
Modified: 2016-07-15 14:46 PDT (History)
3 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
testcase (1.20 KB, application/xhtml+xml)
2006-04-14 10:13 PDT, Steve Speicher
no flags Details
testcase 2 (2.07 KB, application/xhtml+xml)
2006-04-27 18:25 PDT, aaronr
no flags Details
setindex on empty nodeset testcase (2.10 KB, application/xhtml+xml)
2006-05-23 03:49 PDT, Allan Beaufour
no flags Details
Patch (4.21 KB, patch)
2006-05-23 03:53 PDT, Allan Beaufour
no flags Details | Diff | Splinter Review
Patch (5.60 KB, patch)
2006-05-23 07:06 PDT, Allan Beaufour
bugs: review+
aaronr: review+
Details | Diff | Splinter Review

Description Steve Speicher 2006-04-14 10:12:49 PDT
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 2006-04-14 10:13:44 PDT
Created attachment 218443 [details]
testcase
Comment 2 aaronr 2006-04-27 18:14:31 PDT
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 2006-04-27 18:25:27 PDT
Created attachment 220090 [details]
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 2006-05-04 05:22:48 PDT
(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 2006-05-23 02:18:43 PDT
There seems to be generic index problems. Probably regressed from bug 331452.
Comment 6 Allan Beaufour 2006-05-23 03:49:34 PDT
Created attachment 223013 [details]
setindex on empty nodeset testcase

Related problem showing up, when we actually manage to get index to be 0.
Comment 7 Allan Beaufour 2006-05-23 03:53:59 PDT
Created attachment 223014 [details] [diff] [review]
Patch

Also fixes the setindex problem (testcase in attachment 223013 [details]).
Comment 8 Allan Beaufour 2006-05-23 06:55:38 PDT
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 2006-05-23 07:06:43 PDT
Created attachment 223027 [details] [diff] [review]
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] 2006-05-23 07:57:02 PDT
Comment on attachment 223027 [details] [diff] [review]
Patch

r=me, I think :)
Comment 11 Allan Beaufour 2006-05-24 00:32:29 PDT
Fixed on trunk

Note You need to log in before you can comment on or make changes to this bug.