1778718 - SVG used as Data-URI: Style attribute `filter` is ignored

Status: VERIFIED FIXED

(Core :: SVG, defect)

Description

[Jan B.]

Attachment: Screenshot showing expected result on the left and actual result on the right side

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:104.0) Gecko/20100101 Firefox/104.0

Steps to reproduce:

1. create a short HTML file with the following content:
   <img id="image"/>
   <div id="div" style="height: 50px"></div>
   <script type="application/javascript">
   data_url = "data:image/svg+xml,<svg xmlns='http://www.w3.org/2000/svg' height='50px' width='50px' style='filter: hue-rotate(119deg) saturate(75%25) brightness(107%25)%3b'><rect width='20px' height='20px' fill='red'></rect></svg>";
   document.getElementById('image').src = data_url;
   document.getElementById('div').style.setProperty('background-image', url("${data_url}"));
   document.write(navigator.userAgent);
   </script>

2. open the file in Firefox 101 and in Firefox 102, 103 or 104.

Actual results:

In Firefox 101, the rectangles appear in a green color, in Versions 102, 103 and 104, they are red.
Opening the images in a new tab result in green rectangles in any version.

Expected results:

The rectangles should be green in all firefox versions.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Graphics: WebRender' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=400255519b666d678cced4335e32c075f63c937b&tochange=fb4f887f54828081d2498ef4edba340dcc9c3cd3

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

:emilio, since you are the author of the regressor, bug 1423746, could you take a look?
For more information, please visit auto_nag documentation.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Test-case from reporter.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Test-case from reporter.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

We're hitting this code-path and not building the display list for the filter. https://searchfox.org/mozilla-central/rev/5e6e08e53ca4f9f62e19bcbae542eef7824641ed/layout/generic/nsGfxScrollFrame.cpp#3802

Comment 7

[Emilio Cobos Álvarez (:emilio)]

And that's because of this.

Comment 8

[Daniel Holbert [:dholbert]]

Tentatively triaging as S2 since this was a recently-shipped regression, which presumably is affecting content somewhere in-the-wild given that we got a bug report. Would be nice if we could fix the regression in the near term.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1778718 - Clean-up some scroll frame DL building code. r=tnikkel,#gfx-reviewers

This doesn't change behavior but makes the next patch simpler, and makes
the ignore-scrollframe and non-ignore-scrollframe code-paths match.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1778718 - Make filters on the root work when using fallback drawing. r=tnikkel,#gfx-reviewers

This code-path will be hit with the next patch.

Depends on D151473

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1778718 - Make filter/backdrop-filter wrapping work when ignoring a scroll frame. r=tnikkel,#gfx-reviewers

Factor the code to build the top layer and wrapping for filters into a
common function.

Depends on D151474

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/23f4c9e27974
Clean-up some scroll frame DL building code. r=tnikkel

Comment 13

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/23f4c9e27974

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/448330c1255a
Make filters on the root work when using fallback drawing. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/324b78cf12d8
Make filter/backdrop-filter wrapping work when ignoring a scroll frame. r=tnikkel

Comment 15

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/34802 for changes under testing/web-platform/tests

Comment 16

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/448330c1255a
https://hg.mozilla.org/mozilla-central/rev/324b78cf12d8

Comment 17

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox103 to wontfix.

For more information, please visit auto_nag documentation.

Comment 19

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9284900 [details]
Bug 1778718 - Clean-up some scroll frame DL building code. r=tnikkel,#gfx-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: Fixes regression introduced in 102
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See test-case attached to the bug.
• List of other uplifts needed: none
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): It's not a super-trivial patch, but it's well-tested code, and it includes tests.
• String changes made/needed: none
• Is Android affected?: Yes

Comment 20

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9284901 [details]
Bug 1778718 - Make filters on the root work when using fallback drawing. r=tnikkel,#gfx-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: see above, for some reason editing the flags failed.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: see above
• List of other uplifts needed: none
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): see above
• String changes made/needed: none
• Is Android affected?: Yes

Comment 21

[Donal Meehan [:dmeehan]]

Comment on attachment 9284900 [details]
Bug 1778718 - Clean-up some scroll frame DL building code. r=tnikkel,#gfx-reviewers

Approved for 103.0b9, thanks.

Comment 22

[Donal Meehan [:dmeehan]]

Comment on attachment 9284901 [details]
Bug 1778718 - Make filters on the root work when using fallback drawing. r=tnikkel,#gfx-reviewers

Approved for 103.0b9, thanks.

Comment 23

[Donal Meehan [:dmeehan]]

Comment on attachment 9284902 [details]
Bug 1778718 - Make filter/backdrop-filter wrapping work when ignoring a scroll frame. r=tnikkel,#gfx-reviewers

Approved for 103.0b9, thanks.

Comment 24

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/dddc1087d43b
https://hg.mozilla.org/releases/mozilla-beta/rev/aba0ed4e099c
https://hg.mozilla.org/releases/mozilla-beta/rev/6afca8eaf3f4

Comment 25

[Alin Ilea, Desktop QA]

Reproduced the initial issue in Release 102 on Windows 10.
Verified - Fixed in latest Nightly 104.0a1 (2022-07-14) and Beta 103.0b9 using Windows 10, Ubuntu 20 and macOS 12.

Comment 27

[Ryan VanderMeulen [:RyanVM]]

It's not clear to me how commonly this scenario is likely to be encountered in the wild. Do you think this is something we should fix on ESR, Emilio? Or can we live with this bug there? The patches graft cleanly, FWIW.

Comment 28

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9284900 [details]
Bug 1778718 - Clean-up some scroll frame DL building code. r=tnikkel,#gfx-reviewers

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: It's unclear how often this comes up in practice, but yeah given it's a 102 regression might be worth to get this uplifted, also so that it doesn't block other potential fixes.
• User impact if declined: 102 regression fix.
• Fix Landed on Version: 104
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): See beta approval request for rationale. This has been on the tree for almost a month now.

Comment 29

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9284900 [details]
Bug 1778718 - Clean-up some scroll frame DL building code. r=tnikkel,#gfx-reviewers

Approved for 102.2esr.

Comment 30

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr102/rev/3742a0b50220
https://hg.mozilla.org/releases/mozilla-esr102/rev/827275b8032e
https://hg.mozilla.org/releases/mozilla-esr102/rev/7e27d6c58d16

Comment 31

[Alin Ilea, Desktop QA]

Verified - Fixed in 102.2.0esr (build id: 20220808014342) using Windows 10, Ubuntu 20 and macOS 12. The rectangles color is now green.