Closed Bug 1517197 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: SVG, defect, P1)

64 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla66
Tracking Status
firefox-esr60 --- unaffected
firefox64 - wontfix
firefox65 + verified
firefox66 + verified

People

(Reporter: alu, Assigned: jwatt)

References

Details

(Keywords: regression)

Attachments

(1 file)

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.
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
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: 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)

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

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

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.

Pushed by jwatt@jwatt.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/970559b6fcde Fix SVG masking and clipping regression. r=mattwoodrow
Status: NEW → RESOLVED
Closed: 6 years 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+

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)

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.

Attachment

General

Created:
Updated:
Size: