1324809 - CSS filter makes wrapped text disappear

Status: RESOLVED FIXED

(Core :: Layout, defect, P2)

Description

[mattiasw]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0
Build ID: 20161218004028

Steps to reproduce:

1. Go to https://jsfiddle.net/u7v4waq2/1/
2. Hover text that says "Wrapped sentence"


Actual results:

The word "sentence" disappeared.


Expected results:

Content should look the same.

Comment 1

[mattiasw]

Same result using a new Firefox profile.

Comment 2

[YF (Yang)]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=9c43276b9d7de96b983a006a7bf0f47b252ee08b&tochange=5011b0476532fbc5b36a8d75e4618c7d040526cd

Bug 1304011

Comment 3

[u459114]

Clip region at [1] is not correct. Will fix it with a reftest.

[1] http://searchfox.org/mozilla-central/source/layout/svg/nsSVGIntegrationUtils.cpp#720

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

[Tracking Requested - why for this release]: We should avoid shipping Web-facing regressions.

Comment 5

[u459114]

Attachment: Bug 1324809 - Part 1. Fix wrong clip region while painting filter.

Review commit: https://reviewboard.mozilla.org/r/100100/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/100100/

Comment 6

[Julien Cristau [:jcristau]]

Tracking as webcompat regression in 52, thanks David.

Comment 7

[u459114]

I rolled back to 9c43276b9d7d(a version right before Bug 1304011 landed) and visit https://jsfiddle.net/u7v4waq2/1/

hit NS_ASSERTION:
[26562] ###!!! ASSERTION: How did we getting here, then?: 'aFrame->GetParent()->StyleContext()->GetPseudo() == nsCSSAnonBoxes::mozAnonymousBlock'

This assertion is not relative to the patches in Bug 1304011. I will file another bug.

Comment 8

[Markus Stange [:mstange]]

Comment on attachment 8820599 [details]
Bug 1324809 - Part 1. Fix wrong clip region while painting filter.

https://reviewboard.mozilla.org/r/100100/#review100600

Looks good but please add a test.

Comment 9

[u459114]

Before bug 1304011 landed, we clip context only if opacity is not 1.0f [1]. If we add opacity property in https://jsfiddle.net/u7v4waq2/1/

span:hover {
  filter: brightness(1);
  opacity: 0.5;
}

Text will be clipped out even before bug 1304011. BTW, I think we should not clip context in PaintFilter.

[1] https://hg.mozilla.org/mozilla-central/file/a0eb7d02a895/layout/svg/nsSVGIntegrationUtils.cpp#l973

Comment 10

[u459114]

Comment on attachment 8820599 [details]
Bug 1324809 - Part 1. Fix wrong clip region while painting filter.

https://reviewboard.mozilla.org/r/100100/#review100600

I will add in Bug 1325038. Need to fix that NS_ASSERTION which exist longer then bug 1304011

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6a920cc64d11
Part 1. Fix wrong clip region while painting filter. r=mstange

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6a920cc64d11

Comment 13

[u459114]

Attachment: patch for aurora

Comment 14

[u459114]

Attachment: patch for aurora

Comment 15

[Astley Chen (inactive)]

Let's uplift to aurora accordingly.

Comment 16

[u459114]

Try result on aurora
https://treeherder.mozilla.org/#/jobs?repo=try&revision=eefbedb7d827972436e6f62bd971e3d28a13a0a2

Comment 17

[u459114]

Rebase
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5045ec8dbf3303d3d10fbe68917eb9c691bdd44a

Comment 18

[u459114]

Current tip of aurora is very "orange" on "Windows 7 VM opt"
https://treeherder.mozilla.org/#/jobs?repo=try&revision=42ca044f1566d7444e85ed0da6f1eb29449678c1

Comment 19

[u459114]

Comment on attachment 8821186 [details] [diff] [review]
patch for aurora

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1304011
[User impact if declined]:Like the title said:  CSS filter makes wrapped text disappear.
[Is this code covered by automated tests?]:n/a. I explained in comment 10 why can not have a reftest here.
[Has the fix been verified in Nightly?]:yes
[Needs manual test from QE? If yes, steps to reproduce]: n/a
[List of other uplifts needed for the feature/fix]:n/a
[Is the change risky?]:no
[Why is the change risky/not risky?]:Code change is minor. Rollback clipping behavior before patches in Bug 1304011 landed. try result looks good, manual test passed.
[String changes made/needed]:n/a

Comment 20

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8821186 [details] [diff] [review]
patch for aurora

Fix for regression in 52, let's uplift to aurora.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f43d921f0a14