Bugzilla@Mozilla – Bug 574750
Code that iterates through nsCSSValue::Arrays should use size_t for index now
Last modified: 2010-09-27 18:02:58 PDT
Summon comment box
Now that Bug 574059 landed and nsCSSValue::Array::Count() returns a size_t (instead of PRUint16), we need to do a s/PRUint16/size_t/ on code that iterates through these arrays, too. e.g. 278 nsCSSValue::Array *array = aValue.GetArrayValue(); [...] 330 for (PRUint16 index = 1; index < array->Count(); ++index) { 331 AppendCSSValueToString(aProperty, array->Item(index), aResult); 332 333 /* If we're not at the final element, append a comma. */ 334 if (index + 1 != array->Count()) 335 aResult.AppendLiteral(", "); 336 } http://mxr.mozilla.org/mozilla-central/source/layout/style/nsCSSDeclaration.cpp
Created attachment 454105 [details] [review] fix I think this catches all the instances. There are a few that don't matter as much (e.g. in nsStyleAnimation, where we use a PRUint32 as the loop counter, and we're only iterating up to 4), but I figured we might as well fix those too for consistency.
(meant to say: haven't compiled with the attached patch yet -- doing that now)
Comment on attachment 454105 [details] [review] fix This builds fine on my machine. bz, would you mind reviewing this?
Landed on trunk: http://hg.mozilla.org/mozilla-central/rev/619563f026f5 Requesting blocking1.9.1 & 1.9.2, to avoid complications on those branches from comparing PRUint16 list-counter vs. a size_t Count() method as a result of bug 574059.
Marking as security-sensitive at reed's suggestion, to be on the safe side.
Created attachment 454109 [details] [review] fix for 1.9.2 & 1.9.1 Here's the fix backported to 1.9.2 -- basically just removes the chunks for CSS calc() and nsStyleAnimation (which are new on m-c since 1.9.2 branched), and fixes contextual code. This patch applies fine to 1.9.1, too, with fuzz=3 (for minor changes in contextual code).
Comment on attachment 454109 [details] [review] fix for 1.9.2 & 1.9.1 a=LegNeato for 1.9.2.6 and 1.9.2.11. Please submit to mozilla-1.9.2 default and mozilla-1.9.1 default asap, as code freeze is tonight!
Thanks for the quick turnaround on approval! I'm going to wait for this to cycle on mozilla-central first, before landing the 1.9.1 / 1.9.2 version.
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/f2a4c8e82d16 http://hg.mozilla.org/releases/mozilla-1.9.1/rev/025191a13eb7
Is there a means for QA to verify this fix for 1.9.2.7 and 1.9.1.11?
Only via code inspection (making sure the patch landed). I noticed the problem via code inspection in the first place -- we never had a testcase (though it's probably possible to make one).