svg image is not being shown when it's not browser cached yet

VERIFIED FIXED in Firefox 65

Status

()

defect
P1
normal
VERIFIED FIXED
5 months ago
4 months ago

People

(Reporter: alu, Assigned: jwatt)

Tracking

({regression})

64 Branch
mozilla66
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox64- wontfix, firefox65+ verified, firefox66+ verified)

Details

Attachments

(1 attachment)

Reporter

Description

5 months ago
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/71.0.3578.80 Safari/537.36

Steps to reproduce:

1. Open https://www.pixum.de/editor?product_id=5885 on a Desktop Resolution
2. In the bottom preview bar click on different month's of the calendar


Actual results:

You don't see the month's days of the calendar. The svg image is not being shown.

If you click a second time, the days are visible. (Maybe because now the svg image is loaded into browser cache)


Expected results:

The days of the month should be visible. The svg image should be visible.
Reporter

Comment 1

5 months ago
This happens since Firefox 64.
It seems to be related to the browser cache. If the image is not cached yet, it won't be shown. After it is saved into the browser cache, it is visible.
I can reproduce the issue

Andrew, if you have some time, it would be nice to try with mozregression
https://mozilla.github.io/mozregression/quickstart.html
to figure out when this issue has been introduced.
Component: Untriaged → SVG
Product: Firefox → Core

Comment 3

5 months ago
I can also reproduce the issue on Nightly66.0a1 Windows10.

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=0f22d244af4dea74766377b7fc3e3bcd292f9057&tochange=d2218beee05235644592c9c7f36ec8424e1b1f3c

Regressed by: d2218beee052	Jonathan Watt — Bug 1494092. Remove SVGFilterObserverList::IsInObserverLists and related code. r=mattwoodrow

:jwatt
Your patch seems to cause the issue. Can you please look into this?
Blocks: 1494092
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(jwatt)
[Tracking Requested - why for this release]:
Recent regression (with 64)
Assignee

Updated

5 months ago
Assignee: nobody → jwatt
Flags: needinfo?(jwatt)
Priority: -- → P2

[Tracking Requested - why for this release]:

while this is bad, it doesn't seem like something we'd spin another 64 dot release for. Can we get this fixed in 65, jwatt?

Flags: needinfo?(jwatt)
Assignee

Comment 6

4 months ago

Bug 1517458, bug 1519427 and bug 1519838 are other regressions from 1494092.

Flags: needinfo?(jwatt)
Priority: P2 → P1
Assignee

Comment 7

4 months ago

Prior to the patch in bug 1494092 (which caused this regression) we would return aInvalidRegion when there was no filter specified for an element, or that filter wasn't actively being observed. I left behind the code that made us return some other thing in the case that the filter property referenced an invalid filter, only adding an XXX comment noting that the code looked suspicious. If fact that code is completely bogus. We should return aInvalidRegion in both the case that there is no filter specified (what all four regression bugs are hitting), and in the case that the filter is invalid.

Comment 9

4 months ago
Pushed by jwatt@jwatt.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/970559b6fcde
Fix SVG masking and clipping regression. r=mattwoodrow

Comment 10

4 months ago
bugherder
Status: NEW → RESOLVED
Last Resolved: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66

qe-verify+ for this and the other bugs noted in comment 6. Hooray for removing reftest fails-if annotations too. Please nominate this for Beta approval when you get a chance.

Flags: qe-verify+
Flags: needinfo?(jwatt)
Flags: in-testsuite+
Assignee

Comment 12

4 months ago

Comment on attachment 9036480 [details]
Bug 1517197. Make nsSVGIntegrationUtils::AdjustInvalidAreaForSVGEffects return the original area if there are no filter effects. r=mattwoodrow

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1494092

User impact if declined: Painting and hit testing is broken on some content with SVG clip-path or mask. Bug 1517458, bug 1519427 and bug 1519838 are other regressions that have been reported.

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce:

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): The code has changed somewhat between the regression landing and now, but I believe this essentially makes us do what we were doing prior to the regression, except in an edge case where a filter was specified, but couldn't resolve. It seems clear that that was a bug though (unnoticed due to it being an edge case), and even in that case we should be doing what we are doing now.

String changes made/needed: none

Flags: needinfo?(jwatt)
Attachment #9036480 - Flags: approval-mozilla-beta?

I have managed to reproduce this issue on an affected Firefox 65.0b11 build using Windows 10 x64 by following the STR from comment 0.

This issue is verified fixed using Firefox 66.0a1 (20190115221511) on the following OSes: Windows 10 x64, Ubuntu 18.04 x64, Windows 8.1 x64, macOS 10.13.6.

Comment on attachment 9036480 [details]
Bug 1517197. Make nsSVGIntegrationUtils::AdjustInvalidAreaForSVGEffects return the original area if there are no filter effects. r=mattwoodrow

[Triage Comment]
Fixes a number of SVG issues reported in the wild. Approved for 65.0b12.

Attachment #9036480 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

This issue is verified fixed using Firefox 65.0b12 on the following OSes: Windows 10 x64, Ubuntu 18.04 x64, Windows 8.1 x64, macOS 10.11.

Status: RESOLVED → VERIFIED
Flags: qe-verify+

The patch here marked a couple of tests as fails-if(webrender) - is there something we need to do to get these tests passing with WR?

Flags: needinfo?(jwatt)
Assignee

Comment 18

4 months ago

In bug 1494092 I marked those tests as failing, and someone cleaned up the pre-existing WR annotations since I guess there there was no need for them. In this bug I simply put back the WR annotations. If the tests now pass with WR enabled then the annotations can go.

Flags: needinfo?(jwatt)

Ah thanks for the explanation! I guess I just didn't go back far enough in the history.

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