Closed Bug 1324809 Opened 3 years ago Closed 3 years ago

CSS filter makes wrapped text disappear

Categories

(Core :: Layout, defect, P2)

52 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox50 --- unaffected
firefox51 --- unaffected
firefox52 + fixed
firefox53 --- fixed

People

(Reporter: mattiasw, Assigned: u459114)

References

(Blocks 1 open bug)

Details

(Keywords: regression, testcase)

Attachments

(2 files, 1 obsolete file)

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.
Same result using a new Firefox profile.
Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=9c43276b9d7de96b983a006a7bf0f47b252ee08b&tochange=5011b0476532fbc5b36a8d75e4618c7d040526cd

Bug 1304011
Status: UNCONFIRMED → NEW
Has Regression Range: --- → yes
Has STR: --- → yes
Component: Untriaged → Layout
Ever confirmed: true
Keywords: regression, testcase
OS: Unspecified → All
Product: Firefox → Core
Hardware: Unspecified → All
Flags: needinfo?(cku)
Assignee: nobody → cku
Flags: needinfo?(cku)
Blocks: mask-image
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
[Tracking Requested - why for this release]: We should avoid shipping Web-facing regressions.
Tracking as webcompat regression in 52, thanks David.
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.
See Also: → 1325038
Attachment #8820599 - Flags: review?(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.
Attachment #8820599 - Flags: review?(mstange) → review+
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 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
Pushed by cku@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6a920cc64d11
Part 1. Fix wrong clip region while painting filter. r=mstange
https://hg.mozilla.org/mozilla-central/rev/6a920cc64d11
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Attached patch patch for aurora (obsolete) — Splinter Review
Attached patch patch for auroraSplinter Review
Attachment #8821181 - Attachment is obsolete: true
Let's uplift to aurora accordingly.
Priority: -- → P2
Current tip of aurora is very "orange" on "Windows 7 VM opt"
https://treeherder.mozilla.org/#/jobs?repo=try&revision=42ca044f1566d7444e85ed0da6f1eb29449678c1
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
Attachment #8821186 - Flags: approval-mozilla-aurora?
Comment on attachment 8821186 [details] [diff] [review]
patch for aurora

Fix for regression in 52, let's uplift to aurora.
Attachment #8821186 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.