Closed Bug 708155 Opened 12 years ago Closed 12 years ago
Dynamic modifications to 'opacity' on SVG <foreign
Object> don't invalidate correctly
804 bytes, image/svg+xml
881 bytes, image/svg+xml
4.63 KB, patch
|Details | Diff | Splinter Review|
5.72 KB, patch
|Details | Diff | Splinter Review|
No description provided.
(gah, hit 'enter' while typing bug summary. description & testcase coming up)
Summary: Animations on 'opacity' → Animations on 'opacity' of SVG <foreignObject> don't invalidate correctly
STEPS TO REPRODUCE: 1. Load testcase EXPECTED RESULTS: both top half & bottom half of testcase should animate smoothly to lime over 2s, leaving a plain lime rect ACTUAL RESULTS: Bottom half of testcase remains unchanged, until I force a window-refresh by e.g. clicking titlebar or clicking outside of window As noted in the label in the testcase, the bottom half of the testcase is a foreignObject element. FWIW, the actual animation targets a <g> element that is the parent of both halves of the testcase. This also isn't SMIL-dependent, actually -- I can trigger this by modifying the opacity of the <g> element in Web Developer Console. Non-SMIL testcase coming up next.
Reporting with latest nightly: Mozilla/5.0 (X11; Linux x86_64; rv:11.0a1) Gecko/20111206 Firefox/11.0a1
This testcase doesn't use SMIL -- it instead uses script to tweak the opacity after 500ms (as a poor-man's substitute for MozReftestInvalidate). EXPECTED RESULTS: testcase 2 turns entirely lime after 500ms ACTUAL RESULTS: bottom half doesn't change until you force window to invalidate
Assignee: nobody → longsonr
Attachment #579666 - Flags: review?(dholbert+bmo)
Comment on attachment 579666 [details] [diff] [review] patch Looks good! r=me, with one suggestion: >diff --git a/layout/svg/base/src/nsSVGForeignObjectFrame.h b/layout/svg/base/src/nsSVGForeignObjectFrame.h >+ virtual void DidSetStyleContext(nsStyleContext* aOldStyleContext); Could you add the shiny new MOZ_OVERRIDE keyword at the end of that line, before the ";"? That way we'll notice (with a compile error, in some compilers) if the inherited function ever changes its signature without updating this code. You need to #include "mozilla/Attributes.h" to use it. For reference, see: http://mxr.mozilla.org/mozilla-central/source/mfbt/Attributes.h?force=1#134 http://whereswalden.com/2011/11/16/introducing-moz_override-to-annotate-virtual-functions-which-override-base-class-virtual-functions/
Attachment #579666 - Flags: review?(dholbert+bmo) → review+
Summary: Animations on 'opacity' of SVG <foreignObject> don't invalidate correctly → Dynamic modifications to 'opacity' on SVG <foreignObject> don't invalidate correctly
Target Milestone: --- → mozilla11
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
We should revert this from Aurora (Mozilla 11). Daniel, could you do that please?
Sure. (to be clear, this is due to the regression this caused, bug 713413.) Requesting approval to backout from aurora.
Attachment #584779 - Flags: approval-mozilla-aurora?
Comment on attachment 584779 [details] [diff] [review] backout patch for aurora [triage comment] Approved for aurora.
Attachment #584779 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Pushed backout to aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/07fadab6c108
FWIW, this appears to have regressed between Firefox 3.6 & 4.0. Regression range was: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5fda39cd703c&tochange=96de199027d7 Tentatively blaming the biggest bug pushed that day, bug 564991, since there weren't any csets that mentioned SVG, and bug 564991 is invalidation-related.
Verified this using Firefox 12 beta 5 on Windows 7, Mac OS X 10.6 and Ubuntu 11.10. Loaded the test cases attached (testcase1 and testcase2) and both top half & bottom half of the testcases animate smoothly. Mozilla/5.0 (Windows NT 6.1; rv:12.0) Gecko/20100101 Firefox/12.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:12.0) Gecko/20100101 Firefox/12.0 Mozilla/5.0 (X11; Linux i686; rv:12.0) Gecko/20100101 Firefox/12.0
You need to log in before you can comment on or make changes to this bug.