In two messages already today I have criticized people for using the long-winded and inefficient construct
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:value-of select="$i + 1"/>
<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.
Author, XSLT Programmer's Reference and XPath 2.0 Programmer's Reference