Closed
Bug 1318697
Opened 8 years ago
Closed 8 years ago
Elements disappear after sliding in
Categories
(Core :: DOM: Animation, defect, P1)
Core
DOM: Animation
Tracking
()
RESOLVED
FIXED
mozilla53
Tracking | Status | |
---|---|---|
firefox50 | --- | unaffected |
firefox51 | --- | unaffected |
firefox52 | + | fixed |
firefox-esr52 | 52+ | fixed |
firefox53 | + | fixed |
People
(Reporter: freaktechnik, Assigned: hiro)
References
()
Details
(Keywords: dev-doc-complete, regression, site-compat)
Attachments
(4 files, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
text/html
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
text/x-review-board-request
|
birtles
:
review+
jcristau
:
approval-mozilla-release+
jcristau
:
approval-mozilla-esr52+
|
Details |
On http://www.sigareal.ch/de/Angebote.6.html when scrolling on Nightly 53 the parts that get slided-in disappear after reaching 100% opacity until they're moused over.
This does not happen in Firefox 50 without e10s due to add-ons, the panels get displayed as expected.
I've not been able to identify which part of the animation is the problem, but unsetting the "opacity: 1" and then re-enabling it in the style editor will fix the display of all slid-in elements.
Comment 1•8 years ago
|
||
(moving to correct component for diagnosis)
Component: General → Desktop
Product: Web Compatibility → Tech Evangelism
Whiteboard: [needsdiagnosis]
Comment 2•8 years ago
|
||
mozregression got me as far as https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c7d62e6d052c5d2638b08d480a720254ea09ff2d&tochange=cc3ee8d499c511c6fa189fe15b5fea6a57f01382. Looking at that, Bug 1304922 might be the culprit?
Comment 3•8 years ago
|
||
Better STR:
1) Load http://www.sigareal.ch/de/Angebote.6.html
2) Scroll to bottom of page
Expected: elements slide into view and are visible
Actual: by the time you get to the bottom, content is blank
Hiro, can you take a look to see if this is fall-out from Bug 1304922?
Blocks: 1304922
status-firefox50:
--- → unaffected
status-firefox51:
--- → unaffected
status-firefox52:
--- → unaffected
status-firefox53:
--- → affected
tracking-firefox53:
--- → ?
Component: Desktop → Layout
Flags: needinfo?(hiikezoe)
Keywords: regression
Product: Tech Evangelism → Core
Updated•8 years ago
|
Assignee | ||
Comment 4•8 years ago
|
||
Thank you, Mike, for narrowing the cause down. This is actually caused by this change <https://hg.mozilla.org/mozilla-central/rev/01bdca9df2c7>.
This is caused by unnecessary restyle request for Animations level if there are only transitions. I am surprised that unnecessary restyle request causes this kind of issue. I guess there is a bug revealed by the change. Or there is something I am totally missing.
FWIW, attaching patch fixes this issue.
Assignee: nobody → hiikezoe
Flags: needinfo?(hiikezoe)
Assignee | ||
Updated•8 years ago
|
Component: Layout → DOM: Animation
Assignee | ||
Comment 5•8 years ago
|
||
There are opacity and transform transitions.
opacity: 0.5 -> 1
transfrom: translateY(300px) -> translateY(0px) !important
With these transitions, we send a request restyle for Animations level initially, and the result of the request (i.e. an AnimValuesStyleRule) is painted when these transitions finish. I don't know why yet.
Assignee | ||
Comment 6•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0bd288296b82fb136b5e067bc2d29231336438ae
I am still not sure why the unnecessary AnimValuesStyleRule for Animations level is used when the transition finish, but anyway the call stack is:
mozilla::AnimValuesStyleRule::MapRuleInfoInto()
nsRuleNode::WalkRuleTree()
nsRuleNode::GetStyleEffects()
nsStyleContext::DoGetStyleEffects()
nsStyleContext::StyleEffects()
nsStyleContext::CalcStyleDifferenceInternal()
nsStyleContext::CalcStyleDifference()
mozilla::ElementRestyler::CaptureChange()
mozilla::ElementRestyler::RestyleSelf()
mozilla::ElementRestyler::Restyle()
mozilla::ElementRestyler::ComputeStyleChangeFor()
mozilla::RestyleManager::ComputeAndProcessStyleChange()
mozilla::RestyleManager::RestyleElement()
Before the culprit change <https://hg.mozilla.org/mozilla-central/rev/01bdca9df2c7>, we also sent the unnecessary request but it was filtered out in EffectCompositor::ComposeAnimationRule [1].
[1] https://hg.mozilla.org/mozilla-central/rev/01bdca9df2c7#l3.36
We shouldn't, anyway, call RequestRestyle(Layer) for Animations level if there is no animations.
Priority: -- → P1
Assignee | ||
Comment 7•8 years ago
|
||
Now I understand what's going on there.
1) Start two transitions, opacity: 0.5 -> 1; transform: translateY(300px) -> translateY(0px)!important;
2) RequestRestyle(Layer, Animations) and RequestRestyle(Layer, Transitions)
3) ComposeAnimationRule() for both of cascade levels
For opacity: AnimationRule[Animations] = 0.5, AnimationRule[Transitions] = 0.5
4) On initial paint of the transition, AnimationRule[Animations] style is used because animations level is prior to transitions level but we never notice it.
5) While the transitions are running, we don't call RequestRestyle() because both of opacity and transform run on the compositor.
6) When the transitions finish, we call RequestRestyle(Layer, Transitions)[1]
7) The AnimationRule[Animations] is still alive because it's animations level
8) As a result of 6), nsStyleSet::RuleNodeWithReplacement is invoked with eRestyle_CSSTransitions
9) In the RuleNodeWithReplacement, the AnimationRule[Animations] is copied to a new RuleNode[2], on the other hand at this point, a new AnimationRule[Transitions] is null, the new RuleNode has no animation rule for transitions.
[1] https://hg.mozilla.org/mozilla-central/file/0534254e9a40/dom/animation/Animation.cpp#l1233
[2] https://hg.mozilla.org/mozilla-central/file/0534254e9a40/layout/style/nsStyleSet.cpp#l1652
[3] https://hg.mozilla.org/mozilla-central/file/0534254e9a40/layout/style/nsStyleSet.cpp#l1585
Comment 8•8 years ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #7)
> 3) ComposeAnimationRule() for both of cascade levels
> For opacity: AnimationRule[Animations] = 0.5, AnimationRule[Transitions]
> = 0.5
This is the part I don't understand. Why do we make an animations rule for opacity? Do we just always do that because we know transitions will override it?
Assignee | ||
Comment 9•8 years ago
|
||
(In reply to Brian Birtles (:birtles) from comment #8)
> (In reply to Hiroyuki Ikezoe (:hiro) from comment #7)
> > 3) ComposeAnimationRule() for both of cascade levels
> > For opacity: AnimationRule[Animations] = 0.5, AnimationRule[Transitions]
> > = 0.5
>
> This is the part I don't understand. Why do we make an animations rule for
> opacity? Do we just always do that because we know transitions will override
> it?
Regarding the case of ComposeAnimationRule with CascadeLevel::Animations, we need to know transitions level's style for additive or accumulative animations. We can add an assertion that there should be at least one animations' level effect in case of aCascadeLevel == Animations.
Comment 10•8 years ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #9)
> (In reply to Brian Birtles (:birtles) from comment #8)
> > (In reply to Hiroyuki Ikezoe (:hiro) from comment #7)
> > > 3) ComposeAnimationRule() for both of cascade levels
> > > For opacity: AnimationRule[Animations] = 0.5, AnimationRule[Transitions]
> > > = 0.5
> >
> > This is the part I don't understand. Why do we make an animations rule for
> > opacity? Do we just always do that because we know transitions will override
> > it?
>
> Regarding the case of ComposeAnimationRule with CascadeLevel::Animations, we
> need to know transitions level's style for additive or accumulative
> animations. We can add an assertion that there should be at least one
> animations' level effect in case of aCascadeLevel == Animations.
Oh, I think I understand.
If we only have a transition then normally we'd want it to look like this:
CSS transition: opacity 0 -> 1
@ 50%:
transitions level animation-rule: opacity: 0.5
animations level animation-rule: <nothing>
If we have an animation too, then, when producing the animation-level rule, the animation wins over the cascade, and when producing the transition-level rule, we skip the transition so we have:
CSS transition: opacity 0 -> 0.5
Other animation: opacity 0.5 -> 1.0
@ 50%:
transitions level animation-rule: <nothing>
animations level animation-rule: 0.75
But, if we have an additive animation the animation result might end up in the animations level of the cascade:
CSS transition: opacity 0 -> 0.5
Other animation: opacity 0.5 -> 1.0 (additive)
@ 50%:
transitions level animation-rule: <nothing>
animations level animation-rule: 1.0
So, for this last case, we need the transition animation value to build on, but for the first case we don't want it to be added to the rule for the animations level of the cascade.
The solution you proposed makes sense to me provided we do it on a per-property basis. i.e. we should only add properties from transitions to the animations-level rule if we know they are going to be overridden or added to by animations at the animation-level.
Assignee | ||
Comment 11•8 years ago
|
||
Thanks for the summarizing. Your comment noticed me that there is another case that we need to filter effects in KeyframeReadOnly::ComposeStyle(), i.e. compose per properties.
Assignee | ||
Comment 12•8 years ago
|
||
To fix the case in comment 11, I think we need to pass CascadeLevel to ComposeStyle(), and in case of Animations Level, need to pass PropertiesForAnimationsLevel() as well and then compose styles if the composing property is included in PropertiesForAnimationsLevel() in contrast to transition level composing.
Updated•8 years ago
|
Whiteboard: [needsdiagnosis]
Assignee | ||
Comment 14•8 years ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #11)
> Created attachment 8813107 [details]
> Another case without important rule changes
I should note this case in more detail.
There is a 0.3s opacity animation and 1.0s transform transitions. When the opacity animation finishes:
1) We call RequestRestyle(Layer, CascadeLevel::Animations).
2) As a result of 1), we call EffectCompositorAnimationRule(CascadeLevel::Animations).
3) At the moment of 2), the effect of the opacity animation has been already removed.
4) We call KeyframeReadOnly::ComposeStyle() against only the effect of the transform transition
5) As a result of 4), a transformed style at this moment is added in an AnimValueStyleRule of *Animations* level.
6) After 5), we never call RequestRestyle(CascadeLevel::Animations) so the AnimValueStyleRule generated in 5) is cached in nsRuleNode and is shown up after the transition finished.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 17•8 years ago
|
||
Each reftest in each commit fails without corresponding fix.
Here is a try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3ac88da6d0dce07872f4783d3f301ef9a440ede4
Both reftests are in 'R4' on Linux64 opt and 'R21' on Android debug. Unfortunately a very frequent orange (bug 1318953) is also in the 'R4', but these reftests did not fail there.
Comment 18•8 years ago
|
||
mozreview-review |
Comment on attachment 8813955 [details]
Bug 1318697 - Part 1: Skip composing styles for properties depending on cascade level.
https://reviewboard.mozilla.org/r/95258/#review96262
::: dom/animation/EffectCompositor.cpp:619
(Diff revision 1)
> - // priority except properties in propertiesToSkip.
> - const nsCSSPropertyIDSet& propertiesToSkip =
> - aCascadeLevel == CascadeLevel::Animations
> - ? nsCSSPropertyIDSet()
> - : effects->PropertiesForAnimationsLevel();
> + // priority.
> + // In case composing Transitions level, properties in
> + // propertiesForAnimationsLevel will be skiped to compose, on the other hand
> + // in case of Animations level, properties _not_ in
> + // propertiesForAnimationsLevel will be skipped.
> + const nsCSSPropertyIDSet& propertiesForAnimationsLevel =
> + effects->PropertiesForAnimationsLevel();
> for (KeyframeEffectReadOnly* effect : sortedEffectList) {
> - effect->GetAnimation()->ComposeStyle(animationRule, propertiesToSkip);
> + effect->GetAnimation()->ComposeStyle(animationRule,
> + propertiesForAnimationsLevel,
> + aCascadeLevel);
Rather than push all this logic down to KeyframeEffectReadOnly, it seems like it would be a simpler API (and smaller patch!) if we just set up an appropriate nsCSSPropertyIDSet here?
From what I understand we currently have the following arrangement:
Property in animation level?
No Yes
------------------+----------------+----------------
Transitions Level | Add property | Skip property
Animations Level | Add property | Add property
And we want to change the behavior to:
Property in animation level?
No Yes
------------------+----------------+----------------
Transitions Level | Add property | Skip property
Animations Level | Skip property | Add property
Can't we just invert the property set when passing we have the animations level?
We'll need to add a method to nsCSSPropertyIDSet but it should be simple: just doing a bitwise flip of mProperties.
Attachment #8813955 -
Flags: review?(bbirtles)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 21•8 years ago
|
||
mozreview-review-reply |
Comment on attachment 8813955 [details]
Bug 1318697 - Part 1: Skip composing styles for properties depending on cascade level.
https://reviewboard.mozilla.org/r/95258/#review96262
> Rather than push all this logic down to KeyframeEffectReadOnly, it seems like it would be a simpler API (and smaller patch!) if we just set up an appropriate nsCSSPropertyIDSet here?
>
> From what I understand we currently have the following arrangement:
>
> Property in animation level?
> No Yes
> ------------------+----------------+----------------
> Transitions Level | Add property | Skip property
> Animations Level | Add property | Add property
>
> And we want to change the behavior to:
>
> Property in animation level?
> No Yes
> ------------------+----------------+----------------
> Transitions Level | Add property | Skip property
> Animations Level | Skip property | Add property
>
> Can't we just invert the property set when passing we have the animations level?
>
> We'll need to add a method to nsCSSPropertyIDSet but it should be simple: just doing a bitwise flip of mProperties.
Actually I thought the same thing a little, but I did not do it because the inverted nsCSSPropertyIDSet has most of properties in many cases, it was little bit misleading to me (if someone looks into it, etc.). But yeah, I am with the inveted set.
Comment 22•8 years ago
|
||
mozreview-review |
Comment on attachment 8813955 [details]
Bug 1318697 - Part 1: Skip composing styles for properties depending on cascade level.
https://reviewboard.mozilla.org/r/95258/#review96314
I tried to rewrite the commit message to make it more clear (to me at least!).
Does this make sense:
"When we are composing style for the Animations level of the cascade,
if we have transitions-level animations, we *also* need to compose them if we
have animations-level animations that build on top of them using additive or
accumulative composite modes.
However, we should not build those transitions-level animations unless they will
be built on or overridden by a regular animations-level animation. Otherwise
we will end up with transitions-level animations in the animations-level and
while transitions-level will override the animations-level in the cascade,
once the transition finishes there will be nothing to remove the cached
animations-level animation rule."
?
r=me with the following addressed.
Thanks for doing this!
::: layout/reftests/css-transitions/transition-and-animation-with-different-durations.html:24
(Diff revision 2)
> + target.style = "transition: opacity 0.3s; opacity: 1; " +
> + "animation: translate 0.1s;";
300ms is a bit long for a test. Does it still reproduce the problem reliable with a shorter duration? I guess not? Perhaps 200ms with an even shorter animation?
::: layout/style/nsCSSPropertyIDSet.h:70
(Diff revision 2)
> + // Return new nsCSSPropertyIDSet which is the invert of this
> + // nsCSSPropertyIDSet.
"Return a new nsCSSPropertyIDSet which is the inverse of this set."
::: layout/style/nsCSSPropertyIDSet.h:73
(Diff revision 2)
> + nsCSSPropertyIDSet result;
> + for (size_t i = 0; i < mozilla::ArrayLength(mProperties); ++i) {
> + result.mProperties[i] = ~mProperties[i];
> + }
> + return result;
(I was wondering if here was anything in PodOperations we could use for this but it seems like there's not. Also, by my calculation, there are only 10 elements in this array so it's not a big deal.)
Attachment #8813955 -
Flags: review?(bbirtles) → review+
Comment 23•8 years ago
|
||
mozreview-review |
Comment on attachment 8813954 [details]
Bug 1318697 - Part 1: Don't request restyle for animations level if there is no animations' level effect.
https://reviewboard.mozilla.org/r/95256/#review96318
Does the test here still fail if we apply just part 2 and not this part (only its test)?
I'm a little bit hesitant about landing this--at least at the same time. It seems like part 2 is the fix we really need so I'd like to land that first and then add this later as a (possibly slightly risky) optimization.
::: dom/animation/EffectCompositor.cpp:765
(Diff revision 1)
> if (prevCompositorPropertiesWithImportantRules !=
> - compositorPropertiesInSet(propertiesWithImportantRules)) {
> + compositorPropertiesInSet(propertiesWithImportantRules) &&
> + !compositorPropertiesInSet(propertiesForAnimationsLevel).none()) {
I think compositorPropertiesInSet(propertiesForAnimationsLevel).any() would be more clear than a double-negative?
Assignee | ||
Comment 24•8 years ago
|
||
mozreview-review-reply |
Comment on attachment 8813954 [details]
Bug 1318697 - Part 1: Don't request restyle for animations level if there is no animations' level effect.
https://reviewboard.mozilla.org/r/95256/#review96318
No. The part 1 does just stop a unnecessary RequestRestyle(Animations). I will combine the test case in part 1 into part 2.
Assignee | ||
Comment 25•8 years ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #24)
> Comment on attachment 8813954 [details]
> Bug 1318697 - Part 1: Don't request restyle for animations level if there is
> no animations' level effect.
>
> https://reviewboard.mozilla.org/r/95256/#review96318
>
> No. The part 1 does just stop a unnecessary RequestRestyle(Animations). I
> will combine the test case in part 1 into part 2.
And rename part 2 as part 1.
Comment 26•8 years ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #24)
> No. The part 1 does just stop a unnecessary RequestRestyle(Animations). I
> will combine the test case in part 1 into part 2.
Do we need the test in part 1? Is it likely we'll change something that regressesthe test from part 1 but still passes the test in part 2?
Assignee | ||
Comment 27•8 years ago
|
||
(In reply to Brian Birtles (:birtles) from comment #26)
> (In reply to Hiroyuki Ikezoe (:hiro) from comment #24)
> > No. The part 1 does just stop a unnecessary RequestRestyle(Animations). I
> > will combine the test case in part 1 into part 2.
>
> Do we need the test in part 1? Is it likely we'll change something that
> regressesthe test from part 1 but still passes the test in part 2?
Ah, OK. So far, the test is not necessary because we have no way to check RequestRestyle() itself for now.
Comment 28•8 years ago
|
||
mozreview-review |
Comment on attachment 8813954 [details]
Bug 1318697 - Part 1: Don't request restyle for animations level if there is no animations' level effect.
https://reviewboard.mozilla.org/r/95256/#review96334
r=me with the change from comment 23 and without the test
Also, if possible, I'd like to land this separately to make regression tracking easier in case this has unexpected side-effects.
Attachment #8813954 -
Flags: review?(bbirtles) → review+
Assignee | ||
Comment 29•8 years ago
|
||
(In reply to Brian Birtles (:birtles) from comment #22)
> Comment on attachment 8813955 [details]
> Bug 1318697 - Part 2: Skip composing styles for properties depending on
> cascade level.
>
> https://reviewboard.mozilla.org/r/95258/#review96314
>
> I tried to rewrite the commit message to make it more clear (to me at
> least!).
> Does this make sense:
>
> "When we are composing style for the Animations level of the cascade,
> if we have transitions-level animations, we *also* need to compose them if we
> have animations-level animations that build on top of them using additive or
> accumulative composite modes.
>
> However, we should not build those transitions-level animations unless they
> will
> be built on or overridden by a regular animations-level animation. Otherwise
> we will end up with transitions-level animations in the animations-level and
> while transitions-level will override the animations-level in the cascade,
> once the transition finishes there will be nothing to remove the cached
> animations-level animation rule."
I think this sentence is correct (but honestly hard to read). I know, and I understand the root cause of this bug is not easy to understand immediately, animations, animations-level, transtions, transitions level..
Thank you always for corrections of Engrish!
> :::
> layout/reftests/css-transitions/transition-and-animation-with-different-
> durations.html:24
> (Diff revision 2)
> > + target.style = "transition: opacity 0.3s; opacity: 1; " +
> > + "animation: translate 0.1s;";
>
> 300ms is a bit long for a test. Does it still reproduce the problem reliable
> with a shorter duration? I guess not? Perhaps 200ms with an even shorter
> animation?
Actually I tried 200ms locally, if I remember correctly, it sometimes passed *without* fix. But it will not be a problem.
> ::: layout/style/nsCSSPropertyIDSet.h:73
> (Diff revision 2)
> > + nsCSSPropertyIDSet result;
> > + for (size_t i = 0; i < mozilla::ArrayLength(mProperties); ++i) {
> > + result.mProperties[i] = ~mProperties[i];
> > + }
> > + return result;
>
> (I was wondering if here was anything in PodOperations we could use for this
> but it seems like there's not. Also, by my calculation, there are only 10
> elements in this array so it's not a big deal.)
Thanks, I did not know the PodOperations, it can be used somewhere.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Attachment #8813954 -
Attachment is obsolete: true
Assignee | ||
Comment 31•8 years ago
|
||
Comment 32•8 years ago
|
||
Pushed by hiikezoe@mozilla-japan.org:
https://hg.mozilla.org/integration/autoland/rev/940a214b71f0
Part 1: Skip composing styles for properties depending on cascade level. r=birtles
Comment 33•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Updated•8 years ago
|
Version: unspecified → Trunk
Updated•8 years ago
|
QA Whiteboard: [good first verify]
Comment 35•8 years ago
|
||
Are you aware that this issue introduced regression in FF52?
I think this fix should be pushed with next update to v52 as it is ESR release.
You can see that it is already affecting people:
https://bugzilla.mozilla.org/show_bug.cgi?id=1318697
https://support.mozilla.org/t5/Firefox/CSS-and-or-script-animations-handling-broken-with-latest-52-0/td-p/1373309
Comment 36•8 years ago
|
||
*fix* linked issue should be https://bugzilla.mozilla.org/show_bug.cgi?id=1347926
Comment 38•8 years ago
|
||
Based on comment 4 this is a regression from bug 1304922 which landed in 52. It seems the status flags were set incorrectly meaning we shipped this regression to release :(.
Hiro, can you request uplift for this to ESR52? Apparently we're treating ESR 52 differently and are more accepting of patches so it's worth asking.
Flags: needinfo?(hikezoe)
Assignee | ||
Comment 39•8 years ago
|
||
Comment on attachment 8813955 [details]
Bug 1318697 - Part 1: Skip composing styles for properties depending on cascade level.
I did confirm this patch can be applied to esr52.
[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: Users will see garbage paint of CSS transitions.
Fix Landed on Version: Firefox 53
Risk to taking this patch (and alternatives if risky): very low. The patch has been landed for about four months, it's been already beta as well.
String or UUID changes made by this patch: None
Flags: needinfo?(hikezoe)
Attachment #8813955 -
Flags: approval-mozilla-esr52?
Comment 40•8 years ago
|
||
[Tracking Requested - why for this release]: I've got to this bug while investigating the following issue reported today. I could confirm the fix using mozregression. It's an annoying issue for users, so it would be nice to get it fixed with Firefox 52.0.2 if any.
https://mail.mozilla.org/pipermail/firefox-dev/2017-March/005180.html
tracking-firefox52:
--- → ?
Keywords: dev-doc-needed,
site-compat
Comment 41•8 years ago
|
||
I think we need a little post mortem on this bug to figure out how it did NOT make it into Firefox 52. This was a regression that was found and fixed in plenty of time to make Firefox 52.
Do we have a process in place to make sure that regressions go forward to as many releases as possible?
Comment 42•8 years ago
|
||
(In reply to Mike Kaply [:mkaply] from comment #41)
> I think we need a little post mortem on this bug to figure out how it did
> NOT make it into Firefox 52. This was a regression that was found and fixed
> in plenty of time to make Firefox 52.
>
> Do we have a process in place to make sure that regressions go forward to as
> many releases as possible?
The problem was that the status flags were set incorrectly back in comment 3. Everyone just trusted they were correct without double-checking which version the regressing bug landed in, so it didn't occur to anyone that this needed uplift. When the regressing bug was confirmed in comment 4 we should have checked the status flags were correct.
Comment 43•8 years ago
|
||
Posted a quick note for this: https://www.fxsitecompat.com/en-CA/docs/2017/css-transitions-may-not-work-smoothly-when-opacity-is-used-with-other-property/
Keywords: dev-doc-needed → dev-doc-complete
Updated•8 years ago
|
status-firefox-esr52:
--- → affected
Comment 44•8 years ago
|
||
That was my fault, it seems. Sorry. :(
Hi Julien, should we consider including this one in the next 52 dot release? Thanks!
Flags: needinfo?(jcristau)
Comment 46•8 years ago
|
||
<opinion>
This should go in a 52 dot release if it happens and especially in the 52 esr.
This was a "break websites" kind of regression.
</opinion>
Comment 47•8 years ago
|
||
Comment on attachment 8813955 [details]
Bug 1318697 - Part 1: Skip composing styles for properties depending on cascade level.
let's take this animations regression fix for 52.0.2
uplift to release (default branch), esr52 (default and FIREFOX_ESR_52_0_X_RELBRANCH)
Flags: needinfo?(jcristau)
Attachment #8813955 -
Flags: approval-mozilla-release+
Attachment #8813955 -
Flags: approval-mozilla-esr52?
Attachment #8813955 -
Flags: approval-mozilla-esr52+
Updated•8 years ago
|
tracking-firefox-esr52:
--- → 52+
Comment 48•8 years ago
|
||
bugherder uplift |
Flags: in-testsuite+
Comment 49•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-esr52/rev/ee46acc0eeb2 (default - 52.1.0)
https://hg.mozilla.org/releases/mozilla-esr52/rev/afa66bcf9203 (FIREFOX_ESR_52_0_X_RELBRANCH - 52.0.2)
You need to log in
before you can comment on or make changes to this bug.
Description
•