66118 2011-08-11 17:18:15 -0700 HANDLE_ANIMATION_INHERIT_AND_INITIAL macro fails to actually iterate. 2022-02-12 18:09:02 -0800 1 1 1 Unclassified WebKit CSS 528+ (Nightly build) Unspecified Unspecified RESOLVED CONFIGURATION CHANGED InRadar P2 Normal --- 66126 1 macpherson dino bfulgham cmarrin darin dino macpherson mitz simon.fraser webkit-bug-importer oldest_to_newest 450176 0 macpherson 2011-08-11 17:18:15 -0700 Noticed this during refactoring: CSSStyleSelector.cpp:183 list->animation(0)->set##Prop(Animation::initialAnimation##Prop()); \ CSSStyleSelector.cpp:184 for (size_t i = 1; i < list->size(); ++i) \ CSSStyleSelector.cpp:185 list->animation(0)->clear##Prop(); \ As you can see, i is not used in the body of the loop, and instead the 0th element is set repeatedly, which also negates the effect of line 183. Looks like this code has been in since 36634, and I don't know what user-visible bugs this causes, if any. 450180 1 macpherson 2011-08-11 17:20:24 -0700 This bug also appears in the HANDLE_TRANSITION_INHERIT_AND_INITIAL macro. 452026 2 darin 2011-08-16 17:38:26 -0700 Bad news. Somebody should figure out what the symptom is and make a test so we can fix it! 452186 3 dino 2011-08-17 03:00:53 -0700 I'll take a look. 452691 4 macpherson 2011-08-17 16:20:46 -0700 FYI: https://bugs.webkit.org/show_bug.cgi?id=66126 would fix this en passant, but doesn't add tests etc. It may be better to fix/test/close this bug before that goes in, but that's up to reviewers I suppose. 452738 5 dino 2011-08-17 16:57:50 -0700 I agree. We should fix this separately. 452765 6 webkit-bug-importer 2011-08-17 17:44:31 -0700 <rdar://problem/9973753> 452780 7 dino 2011-08-17 18:14:41 -0700 The reason we've never noticed this is that it would mean setting an animation property value to 'initial'. I doubt many people have ever done this. Not only that, since the properties are not inherited you always have the initial value. I'm trying to work out how to write test that hits this code (in which case, why do we even have it?) 456600 8 dino 2011-08-24 18:58:00 -0700 I don't want to hold up this much longer, especially since the linked bug would fix it, but I'm wondering if the fact we never saw this cause breakage is that it is hidden by another bug. This code would only be triggered in the case where an animation property is given the value 'initial'. Animation properties don't inherit, so it isn't a big deal. But I notice this macro does something weird as it is creating the list of animations: for (CSSValueListIterator i = value; i.hasMore(); i.advance()) { \ if (childIndex <= list->size()) \ list->append(Animation::create()); \ mapAnimation##Prop(list->animation(childIndex), i.value()); \ ++childIndex; \ childIndex is 0 when this loop begins. Notice that we append to the list, but then set the value of the first element. We never touch the element we appended. Unless anyone objects, I'm going to close this as WONTFIX and we'll investigate in https://bugs.webkit.org/show_bug.cgi?id=66126 if the above code is buggy, or is doing unnecessary work. 1840761 9 bfulgham 2022-02-12 18:09:02 -0800 Fixed in http://trac.webkit.org/changeset/93848