The default bug view has changed. See this FAQ.

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

VERIFIED FIXED in Firefox 12

Status

()

Core
SVG
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: dholbert, Assigned: Robert Longson)

Tracking

({regression})

Trunk
mozilla12
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox9 affected, firefox10 affected, firefox11 affected, firefox12 verified, status1.9.2 unaffected)

Details

(Whiteboard: [qa!])

Attachments

(4 attachments)

Comment hidden (empty)
(Reporter)

Comment 1

5 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

5 years ago
Created attachment 579558 [details]
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.
(Reporter)

Comment 3

5 years ago
Reporting with latest nightly:
Mozilla/5.0 (X11; Linux x86_64; rv:11.0a1) Gecko/20111206 Firefox/11.0a1
(Reporter)

Comment 4

5 years ago
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
(Assignee)

Comment 5

5 years ago
Created attachment 579666 [details] [diff] [review]
patch
Assignee: nobody → longsonr
Attachment #579666 - Flags: review?(dholbert+bmo)
(Reporter)

Comment 6

5 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

5 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

5 years ago
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
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Updated

5 years ago
Depends on: 713413
(Assignee)

Comment 9

5 years ago
We should revert this from Aurora (Mozilla 11). Daniel, could you do that please?
(Reporter)

Comment 10

5 years ago
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.
Attachment #584779 - Flags: approval-mozilla-aurora?

Comment 11

5 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

5 years ago
Pushed backout to aurora:
 https://hg.mozilla.org/releases/mozilla-aurora/rev/07fadab6c108
status-firefox11: --- → affected
status-firefox12: --- → fixed
(Reporter)

Updated

5 years ago
Target Milestone: mozilla11 → mozilla12
(Reporter)

Comment 13

5 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
(Reporter)

Updated

5 years ago
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
status-firefox12: fixed → verified

Updated

5 years ago
Whiteboard: [qa+] → [qa!]
You need to log in before you can comment on or make changes to this bug.