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

Status: VERIFIED FIXED

(Core :: SVG, defect, P1)

Description

[alu]

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.

Comment 1

[alu]

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.

Comment 2

[Sylvestre Ledru [:Sylvestre]]

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.

Comment 3

[Alice0775 White]

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?

Comment 4

[Sylvestre Ledru [:Sylvestre]]

[Tracking Requested - why for this release]:
Recent regression (with 64)

Comment 5

[Andrew Overholt [:overholt]]

[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?

Comment 6

[Jonathan Watt [:jwatt]]

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

Comment 7

[Jonathan Watt [:jwatt]]

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 8

[Jonathan Watt [:jwatt]]

Attachment: Bug 1517197. Make nsSVGIntegrationUtils::AdjustInvalidAreaForSVGEffects return the original area if there are no filter effects. r=mattwoodrow

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/970559b6fcde
Fix SVG masking and clipping regression. r=mattwoodrow

Comment 10

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/970559b6fcde

Comment 11

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 12

[Jonathan Watt [:jwatt]]

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

Comment 13

[Maria Berlinger [:mberlinger], Fxa/Subplat]

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 14

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e3e8f1521ebd

Comment 16

[Maria Berlinger [:mberlinger], Fxa/Subplat]

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.

Comment 17

[Kartikaya Gupta (email:[e-mail])]

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?

Comment 18

[Jonathan Watt [:jwatt]]

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.

Comment 19

[Kartikaya Gupta (email:[e-mail])]

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