Closed
Bug 708155
Opened 12 years ago
Closed 12 years ago
Dynamic modifications to 'opacity' on SVG <foreignObject> don't invalidate correctly
Categories
(Core :: SVG, defect)
Core
SVG
Tracking
()
VERIFIED
FIXED
mozilla12
Tracking | Status | |
---|---|---|
firefox9 | --- | affected |
firefox10 | --- | affected |
firefox11 | --- | affected |
firefox12 | --- | verified |
status1.9.2 | --- | unaffected |
People
(Reporter: dholbert, Assigned: longsonr)
References
Details
(Keywords: regression, Whiteboard: [qa!])
Attachments
(4 files)
804 bytes,
image/svg+xml
|
Details | |
881 bytes,
image/svg+xml
|
Details | |
4.63 KB,
patch
|
dholbert
:
review+
|
Details | Diff | Splinter Review |
5.72 KB,
patch
|
christian
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Comment 1•12 years ago
|
||
(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
Reporter | ||
Comment 2•12 years ago
|
||
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.
Reporter | ||
Comment 3•12 years ago
|
||
Reporting with latest nightly: Mozilla/5.0 (X11; Linux x86_64; rv:11.0a1) Gecko/20111206 Firefox/11.0a1
Reporter | ||
Comment 4•12 years ago
|
||
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 | ||
Comment 5•12 years ago
|
||
Assignee: nobody → longsonr
Attachment #579666 -
Flags: review?(dholbert+bmo)
Reporter | ||
Comment 6•12 years ago
|
||
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+
Reporter | ||
Updated•12 years ago
|
Summary: Animations on 'opacity' of SVG <foreignObject> don't invalidate correctly → Dynamic modifications to 'opacity' on SVG <foreignObject> don't invalidate correctly
Assignee | ||
Comment 7•12 years ago
|
||
pushed https://hg.mozilla.org/integration/mozilla-inbound/rev/a85e6459fc92
Flags: in-testsuite+
Target Milestone: --- → mozilla11
Comment 8•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a85e6459fc92
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 9•12 years ago
|
||
We should revert this from Aurora (Mozilla 11). Daniel, could you do that please?
Reporter | ||
Comment 10•12 years ago
|
||
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 11•12 years ago
|
||
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+
Reporter | ||
Comment 12•12 years ago
|
||
Pushed backout to aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/07fadab6c108
status-firefox11:
--- → affected
status-firefox12:
--- → fixed
Reporter | ||
Updated•12 years ago
|
Target Milestone: mozilla11 → mozilla12
Reporter | ||
Comment 13•12 years ago
|
||
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.
status1.9.2:
--- → unaffected
status-firefox10:
--- → affected
status-firefox9:
--- → affected
Keywords: regression
Comment 14•12 years ago
|
||
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
Status: RESOLVED → VERIFIED
Updated•12 years ago
|
Whiteboard: [qa+] → [qa!]
You need to log in
before you can comment on or make changes to this bug.
Description
•