1365622 - Canvas.drawImage is not always working for SVG images

Status: RESOLVED DUPLICATE of bug 1345853

(Core :: Graphics: Canvas2D, defect)

Description

[michael.baur]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:53.0) Gecko/20100101 Firefox/53.0
Build ID: 20170504105526

Steps to reproduce:

This fiddle shows the error: https://jsfiddle.net/8gu3prs5/13/

It draws an SVG image which is encoded in a data URI into a canvas element. In addition, it draws some rectangles before and after the drawImage, just as reference. The SVG image contains only one element, a PNG image encoded as data URI.


Actual results:

On *first* run, the image/canvas shows only the rectangles but not the SVG image that is drawn with drawImage. In subsequent runs, the PNG image is typically drawn.

In another test, I added a simple SVG <polygon> (small green triangle) to the SVG image. In this case (https://jsfiddle.net/8gu3prs5/4/), the polygon is drawn but not the PNG image of the SVG image.




Expected results:

Obviously, all parts of the SVG image should be drawn every time.

Comment 1

[michael.baur]

This bug appeared in the image export feature of our graph visualization library. Thus, it affects a rather large user base. This code works fine in Chrome.

Comment 2

[michael.baur]

This seems to work fine in Firefox 45 ESR.

Comment 3

[Sebastian Müller]

What's particular worrying about this regression, is that the feature does work in some cases, but fails in others. 
This is why we did not find out about the bug until a customer pointed us to this problem in one of our demos.
Since there are no timing problems in this code (everything is hard-coded and self-contained in the HTML, thus no network traffic and asynchronous code is involved here, except for the "onload" callback), this looks like a regression in the FF core (SVG?) rendering engine to me. Problematic is that the failure cannot be detected from within JavaScript, no exceptions are thrown or error flags are set and "feature detection" is thus unreliable and very hard to implement. 

If you can tell us how to debug this problem, I can see whether my team can do more investigations.

Comment 4

[Robert Longson [:longsonr]]

Can you use the -moz-regression tool (http://mozilla.github.io/mozregression/) to find a regression range.

Comment 5

[Sebastian Müller]

OK, we used the regression tool and found the following:

The bug was introduced by the "fix" to issue #1341881 in changeset c61b11debf42: since then larger images inside svgs are unconditionally loaded asynchronously so drawing the SVG image could result in the "inner image" not being loaded yet and thus drawing the inner image simply fails silently after that change. This is what we see in the current release version.

However it looks like the fix to issue #1345853 also fixed *this* bug in one of the unreleased versions. That issue was about image patterns not being loaded and the fix was to pass the "synchronous loading flags" down to the image painting method. This very much looks to me like a fix to this bug, and indeed with the builds starting from changeset 87c961bde81e, the bug seems to be fixed already. 

When will this fix be released to stable? #1345853 says 55 which is August. Can it be backported to 54 or even earlier so that our customers don't have to wait so long for the fix?

Comment 6

[Loic]

Testcase: https://jsfiddle.net/8gu3prs5/13/
FF53: broken
FF55: broken (I see no rectangles)

Testcase: https://jsfiddle.net/8gu3prs5/4/
FF53: broken
FF55: fixed

Any thoughts about the 1st testcase? (which looks broken in old versions of Firefox too)

Comment 7

[Sebastian Müller]

@Loic: The first testcase (https://jsfiddle.net/8gu3prs5/13/) is broken in all browsers, because the test is broken itself ("document.getElementbyId is not a function" - the casing of getElementById is broken) - don't worry about "/13/"...
 - "/4/" is the one this bug report is all about.

Comment 8

[michael.baur]

Indeed, this URL should have been https://jsfiddle.net/8gu3prs5/14/. The obvious typo is fixed there.

In any case, as Sebastian states, the testcase /4/ shows the bug more exactly, namely that the problem is the inner PNG image of the SVG and not the SVG as a whole.

Comment 9

[Loic]

I conirm the fix range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=bf0f897261395ed2488f8fef72cf5353da1e105e&tochange=850cf5d6d37dcdb71cc9c22344a6ca33db6a382e

Comment 10

[Robert Longson [:longsonr]]

(In reply to Sebastian Müller from comment #5)
> OK, we used the regression tool and found the following:
> 
> When will this fix be released to stable? #1345853 says 55 which is August.
> Can it be backported to 54 or even earlier so that our customers don't have
> to wait so long for the fix?

You're not reading it right. That's already happened.

Comment 11

[Sebastian Müller]

Sorry to bother you, but can you please help me "reading it right"? The other bug states that it has been fixed for esr52, 53, 54, and 55, with the "53 -> fixed"-comment happening when 53 was still in beta. Now 53 is released, but 53 is still affected by this bug even though it says it got fixed during beta. So which stable version will contain the fix? 53 does not.
The history in the other bug is unclear to me:
first there are fixes for a lot of versions (aurora and beta), then it gets reopened and fixed, for 55, only. So could it be that it would first need to be reopened for 53 and 54, too, so that it can then get fixed for them, too.

Comment 12

[Loic]

Don't expect fix like that in 53, because it's not a blocker.

Comment 13

[Loic]

cjku, do you have an idea why your bugfix in bug 1345853 fixed this issue in 55 but not in 53/54?

Comment 14

[u459114]

That's becasue the patch we land in 55 and 53/54 is not the same. Patches that I landed in 55 did two things:
1. fix a regression I made in Bug 1258510
2. pass SyncDecode flag down to imgLib when need.

53/54 branch patch included #1 only since #2 invoke too much code change, which is not suitable for uplift.

Comment 15

[Loic]

Kohei, can you add a note on fxsitecompat.com about the SVG regression fixed in bug 1345853 in FF55 and partially in 53/54. There are some duplicate reports so the issue is not rare.

Comment 16

[Sebastian Müller]

So to sum it up: this regression, introduced by the one-liner fix in bug 1341881 will only be fixed in v55 in August because it would be "too risky" to backport that one-liner (loading images using FLAG_SYNC_DECODE, again)? 
And now we should live for another 3 months with the broken implementation? 
If so, then please make it clear that this is *not* a duplicate of bug 1345853, which makes it look like it was already fixed, which is *not true* for this bug. Thanks!

Comment 17

[Kohei Yoshino [:kohei]]

I’m sorry but it’s an old bug; I’m not adding a site compat note.