SVG image with animated opacity and a fill filter is blank until hovered

VERIFIED FIXED in Firefox 50

Status

()

Core
Layout
VERIFIED FIXED
a year ago
a year ago

People

(Reporter: florian, Assigned: mattwoodrow)

Tracking

({regression})

Trunk
mozilla50
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 unaffected, firefox48 unaffected, firefox49 unaffected, firefox50+ verified, firefox51 verified, firefox52 verified)

Details

Attachments

(4 attachments)

(Reporter)

Description

a year ago
The animation I'm adding in bug 1275262 stopped working recently. In my fx-team build from 2016-06-10 it worked fine; in Gijs' fx-team build from 2016-06-13 he observed that the camera icon was just blank (I now reproduced the same issue on a current fx-team build), and reappeared after hovering it.
(Reporter)

Comment 1

a year ago
[Tracking Requested - why for this release]: recent regression on Nightly.

mozregression points to https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=658c25a8405745e52f72f1437bd14a5339e4ca9d&tochange=dc033604b24d7f8e87af05265038cc9e9f301438
 -> bug 1263829
tracking-firefox50: --- → ?
Depends on: 1263829
(Reporter)

Updated

a year ago
Summary: SVG image with animated opacity is blank until hovered → SVG image with animated opacity and a fill filter is blank until hovered
(Reporter)

Comment 2

a year ago
Created attachment 8764244 [details]
test.svg
(Reporter)

Comment 3

a year ago
Created attachment 8764245 [details]
test.xul

Steps to reproduce:
1. Download the 2 test files (test.svg and test.xul) into the same folder.
2. In about:config, set dom.allow_XUL_XBL_for_file to true.
3. Load file://.../test.xul

Expected result:
Animated (blinking/pulsing) camera icon.

Actual result:
Blank page until the mouse is moved above it. Once the mouse is moved, the animation starts working. Reloading the page (Cmd+R) makes the page look blank again.


Note: replacing the fill filter with fill="rgb(...)" directly on the SVG path element hides the bug.
(Reporter)

Updated

a year ago
Flags: needinfo?(jwatt)
It seems like dom.allow_XUL_XBL_for_file doesn't work any more. I get the same error message after loading the testcase locally as I do loading it from bmo:

  This page uses an unsupported technology that is no longer
  available by default in Firefox.

  Please contact the website owners to inform them of this problem

Can you create an HTML testcase, Florian?
Flags: needinfo?(jwatt) → needinfo?(florian)
(Reporter)

Comment 5

a year ago
(In reply to Jonathan Watt [:jwatt] (Away Jun. 27 - Jul. 13) from comment #4)
> It seems like dom.allow_XUL_XBL_for_file doesn't work any more.

I used it to get the regression range using mozregression, so I can guarantee you that it works.
Flags: needinfo?(florian)
That doesn't help me at all...
(Reporter)

Comment 7

a year ago
You may want to try with e10s disabled, as I'm not sure how about:config propagates preferences changes to the content process.
(Reporter)

Comment 8

a year ago
Created attachment 8764604 [details]
test.html

I managed to reproduce with an HTML testcase, but it isn't as reliable as the XUL one. To reproduce I click randomly once or twice on the blinking camera icon or outside of it (alternatively), and sometimes the animation stops (the icon disappears) after the click. Moving the mouse makes the animation restart.
(Reporter)

Comment 9

a year ago
The behavior has changed again recently. Now the animation doesn't stop, but the opacity is either 0% or 100%, we don't get anything in between.

mozregression shows https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=067521ccd17907843e1db644ca0f02a1f5a6520a&tochange=eac0c056235ee1174fc092fe159d2d5710331fbd
-> bug 1283827
Blocks: 1283827
Flags: needinfo?(matt.woodrow)
(Assignee)

Comment 10

a year ago
I managed to reproduce this a few times in my Nightly, but can't do so in a local build.

Does this fix the problem?: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e58a5372e62d
Flags: needinfo?(matt.woodrow)
(Assignee)

Updated

a year ago
Flags: needinfo?(florian)
(Reporter)

Comment 11

a year ago
(In reply to Matt Woodrow (:mattwoodrow) from comment #10)
> I managed to reproduce this a few times in my Nightly, but can't do so in a
> local build.
> 
> Does this fix the problem?:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=e58a5372e62d

No, it doesn't (I applied the patch locally to have results quickly without waiting for try).
Flags: needinfo?(florian)
Blocks: 1263829
status-firefox48: --- → unaffected
status-firefox49: --- → unaffected
status-firefox50: --- → affected
No longer depends on: 1263829
Version: unspecified → Trunk
status-firefox47: --- → unaffected
Tracking 50+ for this SVG animation regression.
tracking-firefox50: ? → +
(Reporter)

Updated

a year ago
Blocks: 1206233
(Reporter)

Comment 13

a year ago
(In reply to Matt Woodrow (:mattwoodrow) from comment #10)
> I managed to reproduce this a few times in my Nightly, but can't do so in a
> local build.

Are you still unable to reproduce? Bug 1206233 will make this even more visible.
Flags: needinfo?(matt.woodrow)
(Assignee)

Comment 14

a year ago
Not yet, but I just added a new patch in bug 1287122 that is quite likely related. Does that patch (or more likely, both of them) fix this issue?
Flags: needinfo?(matt.woodrow)
(Reporter)

Comment 15

a year ago
(In reply to Matt Woodrow (:mattwoodrow) from comment #14)
> Not yet, but I just added a new patch in bug 1287122 that is quite likely
> related. Does that patch (or more likely, both of them) fix this issue?

attachment 8774197 [details] [diff] [review] from bug 1287122 fixes the regression from bug 1283827 that I observed in comment 9 here, and reverts the behavior to what was described in comment 1 to 8, ie. the regression from bug 1263829 remains.

When you said "both of them", I assume you meant attachment 8774197 [details] [diff] [review] + the patch in the try build at comment 10. Adding this second patch doesn't (visibly) change the behavior.
(Assignee)

Comment 16

a year ago
Created attachment 8775384 [details] [diff] [review]
Don't skip building layers if we're not actually supposed to consider the opacity

I managed to reproduce it with the XUL testcase, needed a 3rd fix :)
Assignee: nobody → matt.woodrow
Attachment #8775384 - Flags: review?(jwatt)
Comment on attachment 8775384 [details] [diff] [review]
Don't skip building layers if we're not actually supposed to consider the opacity

Review of attachment 8775384 [details] [diff] [review]:
-----------------------------------------------------------------

> I managed to reproduce it with the XUL testcase, needed a 3rd fix :)

\o/
Attachment #8775384 - Flags: review?(jwatt) → review+
(Reporter)

Comment 18

a year ago
Comment on attachment 8775384 [details] [diff] [review]
Don't skip building layers if we're not actually supposed to consider the opacity

Review of attachment 8775384 [details] [diff] [review]:
-----------------------------------------------------------------

I can confirm that applying attachment 8774197 [details] [diff] [review] from bug 1287122 + this patch fixes the animation issue :-). Thanks!

Now it would be great if all these fixes could land before 50 merges to aurora.

::: layout/svg/nsSVGIntegrationUtils.cpp
@@ +632,5 @@
>         aParams.callerPaintsOpacity)) {
>      opacity = 1.0f;
>    }
> +  if (opacity == 0.0f) {
> +    return;

This is |return DrawResult::SUCCESS;| since bug 1258510.
Attachment #8775384 - Flags: feedback+

Comment 19

a year ago
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2bd88c9c8068
Don't skip paint painting SVG effects due to 0 opacity if the opacity is being handled by a separate item. r=jwatt

Comment 20

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/2bd88c9c8068
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox50: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
(Reporter)

Updated

a year ago
Depends on: 1287122
Depends on: 1291522
Verified as fixed on Nightly 52.0a1 -20160920030429, Aurora 51.0a2 - 20160920004004 and Beta 50.0b1-20160920155715.
Status: RESOLVED → VERIFIED
status-firefox50: fixed → verified
status-firefox51: --- → verified
status-firefox52: --- → verified
You need to log in before you can comment on or make changes to this bug.