In two messages already today I have criticized people for using the long-winded and inefficient construct
<xsl:variable name="x">
<xsl:value-of select="y"/>
</xsl:variable>
rather than the shorter and faster
<xsl:variable name="x" select="y"/>
This habit seems to be impossible to eradicate, I've no idea why so many people write 3 lines of code where 1 does the job much better.
In your case it's not only verbose and inefficient, it's wrong. Change
<xsl:with-param name="i">
<xsl:value-of select="$i + 1"/>
</xsl:with-param>
to
<xsl:with-param name="i" select="$i + 1"/>
The resulting variable will be a number rather than a result tree fragment. When used in a predicate [$i], a number is treated as meaning [position()=$i] (which you intended), whereas anything else is treated as a boolean - and in the case of a result tree fragment, the boolean value is always true.
There's another problem though: if $colors is a result tree fragment then in XSLT 1.0 you aren't allowed to probe inside it using a path expression such as $colors/colors/color. It's allowed in XSLT 2.0, and some 1.0 processors let you get away with it, but most don't, and it's an error according to the spec.
Michael Kay
http://www.saxonica.com/
Author, XSLT Programmer's Reference and XPath 2.0 Programmer's Reference