Closed Bug 1323119 Opened 3 years ago Closed 2 years ago

Assertion failure: false (GetDiscretelyAnimatedCSSValue is not implemented yet)

Categories

(Core :: DOM: Animation, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox53 --- wontfix
firefox56 --- wontfix
firefox57 --- fixed
firefox58 --- fixed
firefox59 --- fixed
firefox60 --- fixed
firefox61 --- fixed

People

(Reporter: truber, Assigned: hiro)

References

(Blocks 2 open bugs)

Details

(Keywords: assertion, regression, testcase)

Attachments

(3 files)

Attached file testcase.html
The attached testcase causes the assertion in mozilla-central rev f46f85dcfbc2

Assertion failure: false (GetDiscretelyAnimatedCSSValue is not implemented yet), at src/layout/style/nsHTMLStyleSheet.cpp:119

    #0 0x7fbd80db9d91 in nsHTMLStyleSheet::TableTHRule::GetDiscretelyAnimatedCSSValue(nsCSSPropertyID, nsCSSValue*) src/layout/style/nsHTMLStyleSheet.cpp:119:3
    #1 0x7fbd80dfba0e in nsRuleNode::GetDiscretelyAnimatedCSSValue(nsCSSPropertyID, nsCSSValue*) src/layout/style/nsRuleNode.cpp:10401:9
    #2 0x7fbd80cc0681 in mozilla::StyleAnimationValue::ExtractComputedValue(nsCSSPropertyID, nsStyleContext*, mozilla::StyleAnimationValue&) src/layout/style/StyleAnimationValue.cpp:4708:7
    #3 0x7fbd7d958a6d in mozilla::EffectCompositor::GetBaseStyle(nsCSSPropertyID, nsStyleContext*, mozilla::dom::Element&, mozilla::CSSPseudoElementType) src/dom/animation/EffectCompositor.cpp:917:5
    #4 0x7fbd7d95fecd in mozilla::dom::KeyframeEffectReadOnly::CompositeValue(nsCSSPropertyID, RefPtr<mozilla::AnimValuesStyleRule> const&, mozilla::StyleAnimationValue const&, mozilla::dom::CompositeOperation) src/dom/animation/KeyframeEffectReadOnly.cpp:375:14
    #5 0x7fbd7d94528b in mozilla::dom::KeyframeEffectReadOnly::ComposeStyle(RefPtr<mozilla::AnimValuesStyleRule>&, nsCSSPropertyIDSet const&) src/dom/animation/KeyframeEffectReadOnly.cpp:445:7
    #6 0x7fbd7d944a6f in mozilla::dom::Animation::ComposeStyle(RefPtr<mozilla::AnimValuesStyleRule>&, nsCSSPropertyIDSet const&) src/dom/animation/Animation.cpp:960:7
    #7 0x7fbd7d955875 in mozilla::EffectCompositor::ComposeAnimationRule(mozilla::dom::Element*, mozilla::CSSPseudoElementType, mozilla::EffectCompositor::CascadeLevel, mozilla::TimeStamp) src/dom/animation/EffectCompositor.cpp:700:5
    #8 0x7fbd7d9554eb in mozilla::EffectCompositor::MaybeUpdateAnimationRule(mozilla::dom::Element*, mozilla::CSSPseudoElementType, mozilla::EffectCompositor::CascadeLevel, nsStyleContext*) src/dom/animation/EffectCompositor.cpp:370:3
    #9 0x7fbd7d955bdd in mozilla::EffectCompositor::GetAnimationRule(mozilla::dom::Element*, mozilla::CSSPseudoElementType, mozilla::EffectCompositor::CascadeLevel, nsStyleContext*) src/dom/animation/EffectCompositor.cpp:406:3
    #10 0x7fbd80e1845e in nsStyleSet::RuleNodeWithReplacement(mozilla::dom::Element*, mozilla::dom::Element*, nsRuleNode*, mozilla::CSSPseudoElementType, nsRestyleHint) src/layout/style/nsStyleSet.cpp:1566:34
    #11 0x7fbd80e18b8f in nsStyleSet::ResolveStyleWithReplacement(mozilla::dom::Element*, mozilla::dom::Element*, nsStyleContext*, nsStyleContext*, nsRestyleHint, unsigned int) src/layout/style/nsStyleSet.cpp:1676:5
    #12 0x7fbd80f10c7b in mozilla::ElementRestyler::RestyleSelf(nsIFrame*, nsRestyleHint, unsigned int*, nsTArray<mozilla::ElementRestyler::SwapInstruction>&) src/layout/base/RestyleManager.cpp:2816:11
    #13 0x7fbd80f0e3af in mozilla::ElementRestyler::Restyle(nsRestyleHint) src/layout/base/RestyleManager.cpp:2139:7
    #14 0x7fbd80f19768 in mozilla::ElementRestyler::ComputeStyleChangeFor(nsIFrame*, nsStyleChangeList*, nsChangeHint, mozilla::RestyleTracker&, nsRestyleHint, mozilla::RestyleHintData const&, nsTArray<mozilla::ElementRestyler::ContextToClear>&, nsTArray<RefPtr<nsStyleContext> >&) src/layout/base/RestyleManager.cpp:3393:7
    #15 0x7fbd80f03189 in mozilla::RestyleManager::ComputeAndProcessStyleChange(nsIFrame*, nsChangeHint, mozilla::RestyleTracker&, nsRestyleHint, mozilla::RestyleHintData const&) src/layout/base/RestyleManager.cpp:3803:3
    #16 0x7fbd80f0261c in mozilla::RestyleManager::RestyleElement(mozilla::dom::Element*, nsIFrame*, nsChangeHint, mozilla::RestyleTracker&, nsRestyleHint, mozilla::RestyleHintData const&) src/layout/base/RestyleManager.cpp:152:5
    #17 0x7fbd80f21bba in mozilla::RestyleTracker::ProcessOneRestyle(mozilla::dom::Element*, nsRestyleHint, nsChangeHint, mozilla::RestyleHintData const&) src/layout/base/RestyleTracker.cpp:97:5
    #18 0x7fbd80f1fbed in mozilla::RestyleTracker::DoProcessRestyles() src/layout/base/RestyleTracker.cpp:266:9
    #19 0x7fbd80f061ba in mozilla::RestyleManager::ProcessPendingRestyles() src/layout/base/RestyleManager.cpp:834:3
    #20 0x7fbd80ed970b in mozilla::PresShell::FlushPendingNotifications(mozilla::ChangesToFlush) src/layout/base/PresShell.cpp:4115:9
I think we should implement TableTHRule::GetDiscretelyAnimatedCSSValue and other GetDiscretelyAnimatedCSSValue to get the base style.  I guess we need to walk up tree rules to find the style.  I wondered how CSS transition handle these cases, but currently we do ignore discrete properties for transitions.
(In reply to Hiroyuki Ikezoe (:hiro) from comment #1)
> I think we should implement TableTHRule::GetDiscretelyAnimatedCSSValue and
> other GetDiscretelyAnimatedCSSValue to get the base style.  I guess we need
> to walk up tree rules to find the style. 

I'd like to fix this property in stylo, since we need to use each member variable which represents each CSS value respectively, e.g. mMaskType in nsStyleSVGReset, it will be some amount of work.
Blocks: 1305325
Priority: -- → P3
Depends on: 1320854
Blocks: 1334591
Attached file Testcase
Additional testcase which triggers the assertion "GetDiscretelyAnimatedCSSValue is not implemented yet".
Unsurprisingly it sounds like (given comment 2), this doesn't reproduce when Stylo is enabled.
Flags: in-testsuite?
Has Regression Range: --- → yes
Whiteboard: [fixed by stylo]
Comment on attachment 8966090 [details]
Bug 1323119 - Add a test case that had caused an assertion on the old style system.

https://reviewboard.mozilla.org/r/234832/#review240442

::: dom/animation/test/crashtests/1323119-1.html:6
(Diff revision 1)
> +<!DOCTYPE html>
> +<html class="reftest-wait">
> +<body>
> +<script>
> +addEventListener("DOMContentLoaded", function (){
> +  let a=document.createElement("th");

I wonder if we don't need the coding style in here?
(Although there were similar codes in crashtests.)
If we need, please add one space both side of "=".
Attachment #8966090 - Flags: review?(dakatsuka) → review+
Pushed by hikezoe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c1942e1742f2
Add a test case that had caused an assertion on the old style system. r=daisuke
https://hg.mozilla.org/mozilla-central/rev/c1942e1742f2
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Assignee: nobody → hikezoe
Flags: in-testsuite? → in-testsuite+
Whiteboard: [fixed by stylo]
You need to log in before you can comment on or make changes to this bug.