Gradient of svg inside blob not rendering in firefox 59.0.1 (or clip-path, etc. using local URI reference)

VERIFIED FIXED in Firefox 60

Status

()

defect
VERIFIED FIXED
a year ago
a year ago

People

(Reporter: mcr, Assigned: baku)

Tracking

({regression})

59 Branch
mozilla61
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox59 wontfix, firefox60 verified, firefox61 verified)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

a year ago
User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.162 Safari/537.36

Steps to reproduce:

Looks like https://bugzilla.mozilla.org/show_bug.cgi?id=841920

Open this jsfiddle : https://jsfiddle.net/8f8f685j/  (copied from http://www.snappymaria.com/misc/svg-id-test/ )
Click on "Open as blob..." to show svg inside blobs

OS : Windows 10 64 bits
Firefox : 59.0.1 64 bits and also firefox developer edition 60.0b4
It worked in 58.0.2


Actual results:

the svg with gradient are not rendered in blobs (only 2 images out of 4 are renderered correctly)


Expected results:

The 4 images should have been displayed correctly

Updated

a year ago
Component: Untriaged → SVG
Keywords: regression
Product: Firefox → Core

Comment 1

a year ago
regression-window
Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=1f7a23820597&tochange=d860a3b8ea29

Regressed by:
d860a3b8ea29	Andrea Marchesini — Bug 1416193 - Cloned nsHostObjectURI objects should be stored together with their BlobImpl by nsHostObjectProtocolHandler, r=valentin

@:baku,
Could you look into this?
Blocks: 1416193
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(amarchesini)
We might fix this in Nightly with bug 1416791, but we probably need a different fix for beta.
Duplicate of this bug: 1446634
FWIW, we have a further-reduced testcase from a duplicate bug (using clip-path to reference a local URI), here:
https://bugzilla.mozilla.org/attachment.cgi?id=8959793
Duplicate of this bug: 1446820
Summary: Gradient of svg inside blob not rendering in firefox 59.0.1 → Gradient of svg inside blob not rendering in firefox 59.0.1 (or clip-path, etc. using local URI reference)
Assignee

Comment 6

a year ago
Valentin, do you want to take this bug and maybe uplift your patch (or something similar) to beta?
Flags: needinfo?(amarchesini)
(In reply to Andrea Marchesini [:baku] from comment #6)
> Valentin, do you want to take this bug and maybe uplift your patch (or
> something similar) to beta?

I just tested the attachment with my patch from bug 1416791 and it doesn't really fix the problem.
Not sure what the issue is though. I would have thought that the problem is that we use newRef instead of the spec at [1]
But it seems not.

[1] https://searchfox.org/mozilla-central/rev/877c99c523a054419ec964d4dfb3f0cadac9d497/dom/file/nsHostObjectURI.cpp#190

Updated

a year ago
See Also: → 1280584

Updated

a year ago
Blocks: 841920
setting "in-testsuite" -- when we fix this, let's be sure we get an automated test, so that we don't break this again. (This bug is basically bug 841920 being resurrected, after previously having been fixed.)
Flags: in-testsuite?
Comment hidden (obsolete)
Comment hidden (obsolete)
Assignee

Updated

a year ago
Assignee: nobody → amarchesini
Assignee

Comment 11

a year ago
The issue here is that:
        <rect x="0" y="0" width="100" height="100" fill="#ff9933" clip-path="url(#clip)"/>

wants to set a ref for the cloned blob URL, but that is set as immutable and the rendering fails.
It makes sense to call NS_TryToSetImmutable() when a blobURL is created or cloned, but why should not allow the changing of ref or the query part? Should we maybe have a 2 levels for mMutable?
Flags: needinfo?(valentin.gosu)
URLs are basically immutable right now. Cloning actually creates a new URI. So... I don't think you need to call NS_TryToSetImmutable.
After I make them threadsafe, I intend to remove mMutable completely.
Flags: needinfo?(valentin.gosu)
Assignee

Comment 13

a year ago
Posted patch blobURL.patchSplinter Review
Attachment #8960814 - Flags: review?(valentin.gosu)
Attachment #8960814 - Flags: review?(valentin.gosu) → review+
Backed out for /FileAPI/url/url-with-fetch.any.worker.html unexpected pass

backout link : https://hg.mozilla.org/integration/mozilla-inbound/rev/9dac573da4921c621d3679a942a1f0a43aaf5cba

push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=93cd9d50d653ab87e0dd8251950176e574c86c8e

failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=169387744&repo=mozilla-inbound&lineNumber=2695

00:47:57     INFO - TEST-START | /FileAPI/url/url-with-fetch.any.worker.html
00:47:57     INFO - PID 1753 | ++DOCSHELL 0x1298b3800 == 2 [pid = 1755] [id = {f2f8aace-d748-014b-b32a-8885a920af3f}]
00:47:57     INFO - PID 1753 | ++DOMWINDOW == 5 (0x1298dac00) [pid = 1755] [serial = 5] [outer = 0x0]
00:47:58     INFO - PID 1753 | ++DOMWINDOW == 6 (0x1298db400) [pid = 1755] [serial = 6] [outer = 0x1298dac00]
00:47:58     INFO - PID 1753 | ++DOMWINDOW == 7 (0x11dd09800) [pid = 1755] [serial = 7] [outer = 0x1298dac00]
00:47:58     INFO - PID 1753 | ++DOCSHELL 0x128f1f000 == 3 [pid = 1755] [id = {3c9a1641-1894-9247-8a85-d26334105ef1}]
00:47:58     INFO - PID 1753 | ++DOMWINDOW == 8 (0x1298e1000) [pid = 1755] [serial = 8] [outer = 0x0]
00:47:58     INFO - PID 1753 | ++DOMWINDOW == 9 (0x1298e1800) [pid = 1755] [serial = 9] [outer = 0x1298e1000]
00:47:58     INFO - PID 1753 | ++DOMWINDOW == 10 (0x1298e6800) [pid = 1755] [serial = 10] [outer = 0x1298e1000]
00:47:58     INFO - PID 1753 | [Child 1755, Main Thread] WARNING: NS_ENSURE_TRUE(!(err)) failed: file /builds/worker/workspace/build/src/toolkit/xre/nsXREDirProvider.cpp, line 1413
00:47:58     INFO - PID 1753 | [Child 1755, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file /builds/worker/workspace/build/src/toolkit/xre/nsXREDirProvider.cpp, line 1548
00:47:59     INFO - PID 1753 | [Child 1755, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x805303F4: file /builds/worker/workspace/build/src/dom/fetch/FetchDriver.cpp, line 554
00:47:59     INFO - PID 1753 | [Child 1755, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x805303F4: file /builds/worker/workspace/build/src/dom/fetch/FetchDriver.cpp, line 554
00:47:59     INFO - PID 1753 | [Child 1755, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x805303F4: file /builds/worker/workspace/build/src/dom/fetch/FetchDriver.cpp, line 554
00:47:59     INFO - PID 1753 | [Child 1755, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x805303F4: file /builds/worker/workspace/build/src/dom/fetch/FetchDriver.cpp, line 554
00:47:59     INFO - TEST-UNEXPECTED-OK | /FileAPI/url/url-with-fetch.any.worker.html | expected CRASH
00:47:59     INFO - TEST-INFO expected CRASH | took 1503ms
00:47:59     INFO - PID 1753 | Unable to read VR Path Registry from /Users/cltbld/Library/Application Support/OpenVR/.openvr/openvrpaths.vrpath
00:47:59     INFO - PID 1753 | [Child 1755, Main Thread] WARNING: nsAppShell::Exit() called redundantly: file /builds/worker/workspace/build/src/widget/cocoa/nsAppShell.mm, line 738
00:47:59     INFO - PID 1753 | [Child 1755, Main Thread] WARNING: A runnable was posted to a worker that is already shutting down!: file /builds/worker/workspace/build/src/dom/workers/WorkerPrivate.cpp, line 1541
00:47:59     INFO - PID 1753 | [Child 1755, Main Thread] WARNING: Failed to dispatch offline status change event!: file /builds/worker/workspace/build/src/dom/workers/WorkerPrivate.cpp, line 2061
00:48:00     INFO - PID 1753 | [Child 1755, Main Thread] WARNING: NS_ENSURE_TRUE(maybeContext) failed: file /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp, line 809
00:48:00     INFO - PID 1753 | 2018-03-21 00:48:00.030 plugin-container[1756:9749] *** CFMessagePort: bootstrap_register(): failed 1100 (0x44c) 'Permission denied', port = 0x8f4f, name = 'com.apple.tsm.portname'
00:48:00     INFO - PID 1753 | See /usr/include/servers/bootstrap_defs.h for the error codes.
00:48:00     INFO - PID 1753 | --DOCSHELL 0x121922000 == 3 [pid = 1753] [id = {3f480d33-ee22-5e4f-a89e-7bc423001ecc}]
00:48:00     INFO - PID 1753 | --DOCSHELL 0x12c425800 == 2 [pid = 1753] [id = {8db0677a-7069-834b-90cd-4f76d0581f15}]
00:48:00     INFO - PID 1753 | [Child 1756, Main Thread] WARNING: nsAppShell::Exit() called redundantly: file /builds/worker/workspace/build/src/widget/cocoa/nsAppShell.mm, line 738
00:48:00     INFO - PID 1753 | 
00:48:00     INFO - PID 1753 | ###!!! [Parent][RunMessage] Error: Channel closing: too late to send/recv, messages will be lost
00:48:00     INFO - PID 1753 | 
00:48:00     INFO - PID 1753 | [Child 1756, Main Thread] WARNING: NS_ENSURE_TRUE(maybeContext) failed: file /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp, line 809
00:48:00     INFO - PID 1753 | [Child 1756, Main Thread] WARNING: '!gThread', file /builds/worker/workspace/build/src/xpcom/threads/nsTimerImpl.cpp, line 399
00:48:00     INFO - PID 1753 | [Child 1756, Main Thread] WARNING: '!gThread', file /builds/worker/workspace/build/src/xpcom/threads/nsTimerImpl.cpp, line 399
00:48:00     INFO - PID 1753 | [Child 1756, Main Thread] WARNING: '!gThread', file /builds/worker/workspace/build/src/xpcom/threads/nsTimerImpl.cpp, line 399
00:48:00     INFO - PID 1753 | [Child 1756, Main Thread] WARNING: '!gThread', file /builds/worker/workspace/build/src/xpcom/threads/nsTimerImpl.cpp, line 399
00:48:00     INFO - PID 1753 | nsStringStats
00:48:00     INFO - PID 1753 |  => mAllocCount:           5622
00:48:00     INFO - PID 1753 |  => mReallocCount:          229
00:48:00     INFO - PID 1753 |  => mFreeCount:            5622
00:48:00     INFO - PID 1753 |  => mShareCount:           4491
00:48:00     INFO - PID 1753 |  => mAdoptCount:            181
00:48:00     INFO - PID 1753 |  => mAdoptFreeCount:        181
00:48:00     INFO - PID 1753 |  => Process ID: 1756, Thread ID: 140735284351744
00:48:00     INFO - PID 1753 | *** UTM:SVC TimerManager:registerTimer called after profile-before-change notification. Ignoring timer registration for id: telemetry_modules_ping
00:48:00     INFO - PID 1753 | --DOCSHELL 0x1228e4000 == 2 [pid = 1755] [id = {3efc1e75-6644-d24f-9c13-72a76fe5350e}]
Flags: needinfo?(amarchesini)
Assignee

Updated

a year ago
Flags: needinfo?(amarchesini)

Comment 17

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c4d938fa8c8b
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61

Comment 18

a year ago
OK, this is indeed solved on the nightly build.

when will this fix be available to not nightly users? our site suffered badly from this.
(In reply to vzaidman from comment #18)
> when will this fix be available to not nightly users?

Likely in Firefox 60 (which is currently "Firefox beta", goes to release on 2018-05-07).  That version doesn't have the fix yet, but you'll see this patch go through an approval process to get backported to that branch over the next few days, I imagine, and then it'll be available for Firefox 60 Beta users.

Setting needinfo=baku to be sure this is on his radar to get uplift approval.
Flags: needinfo?(amarchesini)
Assignee

Comment 20

a year ago
Comment on attachment 8960814 [details] [diff] [review]
blobURL.patch

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1416193
[User impact if declined]: blobURLs do not work with some SVG transformations.
[Is this code covered by automated tests?]: none
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: STR included + tests. 
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: low
[Why is the change risky/not risky?]: The issue is that blobURLs are marked as immutable. This is wrong. The patch removes this limitation.
[String changes made/needed]: none
Flags: needinfo?(amarchesini)
Attachment #8960814 - Flags: approval-mozilla-beta?
(In reply to Daniel Holbert [:dholbert] from comment #8)
> setting "in-testsuite" -- when we fix this, let's be sure we get an
> automated test, so that we don't break this again. (This bug is basically
> bug 841920 being resurrected, after previously having been fixed.)

It doesn't look like that's happened yet (adding an automated test)?  Any particular problem blocking that?
Comment on attachment 8960814 [details] [diff] [review]
blobURL.patch

fix an issue with blobs, beta60+
Attachment #8960814 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+

Comment 24

a year ago
Build ID: 20180329154119
User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:59.0) Gecko/20100101 Firefox/59.0

Verified as fixed on Firefox Nightly 61.0a1 and Firefox 60.0b8 on Windows 10 x 64, Windows 7 x32, Mac OS X 10.12 and Ubuntu 16.04 x64.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.