Closed Bug 708155 Opened 8 years ago Closed 8 years ago

Dynamic modifications to 'opacity' on SVG <foreignObject> don't invalidate correctly

Categories

(Core :: SVG, defect)

defect
Not set

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)

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
Attached image testcase 1
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
Attached image testcase 2 (no SMIL)
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
Attached patch patchSplinter Review
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
pushed https://hg.mozilla.org/integration/mozilla-inbound/rev/a85e6459fc92
Flags: in-testsuite+
Target Milestone: --- → mozilla11
https://hg.mozilla.org/mozilla-central/rev/a85e6459fc92
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Depends on: 713413
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+
Target Milestone: mozilla11 → mozilla12
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.
Blocks: 564991
Whiteboard: [qa+]
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
Whiteboard: [qa+] → [qa!]
You need to log in before you can comment on or make changes to this bug.