As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact
Last Comment Bug 708155 - Dynamic modifications to 'opacity' on SVG <foreignObject> don't invalidate correctly
: Dynamic modifications to 'opacity' on SVG <foreignObject> don't invalidate co...
: regression
Product: Core
Classification: Components
Component: SVG (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla12
Assigned To: Robert Longson
: Jet Villegas (:jet)
Depends on: 713413
Blocks: 564991
  Show dependency treegraph
Reported: 2011-12-06 17:59 PST by Daniel Holbert [:dholbert]
Modified: 2012-04-13 06:32 PDT (History)
5 users (show)
longsonr: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase 1 (804 bytes, image/svg+xml)
2011-12-06 18:03 PST, Daniel Holbert [:dholbert]
no flags Details
testcase 2 (no SMIL) (881 bytes, image/svg+xml)
2011-12-06 18:08 PST, Daniel Holbert [:dholbert]
no flags Details
patch (4.63 KB, patch)
2011-12-07 05:19 PST, Robert Longson
dholbert: review+
Details | Diff | Splinter Review
backout patch for aurora (5.72 KB, patch)
2011-12-29 10:00 PST, Daniel Holbert [:dholbert]
christian: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description User image Daniel Holbert [:dholbert] 2011-12-06 17:59:13 PST

Comment 1 User image Daniel Holbert [:dholbert] 2011-12-06 17:59:55 PST
(gah, hit 'enter' while typing bug summary. description & testcase coming up)
Comment 2 User image Daniel Holbert [:dholbert] 2011-12-06 18:03:39 PST
Created attachment 579558 [details]
testcase 1

 1. Load testcase

EXPECTED RESULTS: both top half & bottom half of testcase should animate smoothly to lime over 2s, leaving a plain lime rect

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.
Comment 3 User image Daniel Holbert [:dholbert] 2011-12-06 18:03:59 PST
Reporting with latest nightly:
Mozilla/5.0 (X11; Linux x86_64; rv:11.0a1) Gecko/20111206 Firefox/11.0a1
Comment 4 User image Daniel Holbert [:dholbert] 2011-12-06 18:08:15 PST
Created attachment 579559 [details]
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
Comment 5 User image Robert Longson 2011-12-07 05:19:04 PST
Created attachment 579666 [details] [diff] [review]
Comment 6 User image Daniel Holbert [:dholbert] 2011-12-07 09:54:12 PST
Comment on attachment 579666 [details] [diff] [review]

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:
Comment 8 User image Matt Brubeck (:mbrubeck) 2011-12-17 09:34:39 PST
Comment 9 User image Robert Longson 2011-12-29 04:00:26 PST
We should revert this from Aurora (Mozilla 11). Daniel, could you do that please?
Comment 10 User image Daniel Holbert [:dholbert] 2011-12-29 10:00:03 PST
Created attachment 584779 [details] [diff] [review]
backout patch for aurora

Sure.  (to be clear, this is due to the regression this caused, bug 713413.)

Requesting approval to backout from aurora.
Comment 11 User image christian 2011-12-29 14:48:41 PST
Comment on attachment 584779 [details] [diff] [review]
backout patch for aurora

[triage comment]
Approved for aurora.
Comment 12 User image Daniel Holbert [:dholbert] 2011-12-29 15:36:55 PST
Pushed backout to aurora:
Comment 13 User image Daniel Holbert [:dholbert] 2011-12-29 16:05:33 PST
FWIW, this appears to have regressed between Firefox 3.6 & 4.0.

Regression range was:

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.
Comment 14 User image Simona B [:simonab ] 2012-04-13 06:30:56 PDT
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

Note You need to log in before you can comment on or make changes to this bug.