Closed Bug 1324524 Opened 3 years ago Closed 3 years ago

Table cells with opacity < 1 can't have collapsed borders

Categories

(Core :: Layout: Tables, defect)

49 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox50 --- wontfix
firefox51 - wontfix
firefox52 - fixed
firefox-esr52 --- fixed
firefox53 + fixed
firefox54 --- fixed

People

(Reporter: epinal99-bugzilla2, Assigned: mattwoodrow)

References

()

Details

(Keywords: regression, Whiteboard: [mozfr-community])

Attachments

(1 file)

Issue reported here: https://forums.mozfr.org/viewtopic.php?f=5&t=131923

Testcase: http://codepen.io/X-Raym/pen/VmqqLr

Regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=9915ff6e7bf778341f46b5bc095ec46deb673416&tochange=952eae508967ae46d36909d179159f2a2205169c

Andrew Comminos — Bug 1276734 - Use nsDisplayBackgroundImage for table cells without collapsed borders. r=mattwoodrow

Andrew, could you look at this regression, please.
Whiteboard: [mozfr-community]
With only a week left before the 51RC build, marking this fix-optional for 51/52.
Matt, can you help out here? Asking you since there's no triage owner here and you reviewed the original patch. Thanks!
Flags: needinfo?(matt.woodrow)
Attached patch border-collapseSplinter Review
It's only safe to use 'normal' background items if we don't have border-collapse. The existing branch had multiple conditions that would allow it to be taken (and opacity triggered one of them) even if it didn't have border-collapse.

It's possible that the event handling condition could use the normal background items, but it's not important for the use case that this was added for, and I'm not confident in our testing of event handling within tables to risk it.
Assignee: nobody → matt.woodrow
Flags: needinfo?(matt.woodrow)
Flags: needinfo?(andrew)
Attachment #8834768 - Flags: review?(tnikkel)
Attachment #8834768 - Flags: review?(tnikkel) → review+
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/88b5a981e275
Make sure we use nsDisplayTableCellbackground for tables with border-collapse. r=tnikkel
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/8160157dcec0 - Win8 was consistently max difference: 2, number of differing pixels: 8258, but figuring out what random pile of %20s amounts to Win8 is above my pay grade.
Flags: needinfo?(matt.woodrow)
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/71227c92ce4f
Make sure we use nsDisplayTableCellbackground for tables with border-collapse. r=tnikkel
Flags: needinfo?(matt.woodrow)
https://hg.mozilla.org/mozilla-central/rev/71227c92ce4f
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Please request Aurora/Beta approval on this when you get a chance.
Flags: needinfo?(matt.woodrow)
Flags: in-testsuite+
Comment on attachment 8834768 [details] [diff] [review]
border-collapse

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1276734
[User impact if declined]: Incorrect table rendering when combining stacking-contexts (opacity) and border-collapse.
[Is this code covered by automated tests?]: Yes, new test added.
[Has the fix been verified in Nightly?]: No
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Just reverts back to the old behaviour in some cases.
[String changes made/needed]:
Flags: needinfo?(matt.woodrow)
Attachment #8834768 - Flags: approval-mozilla-beta?
Attachment #8834768 - Flags: approval-mozilla-aurora?
Comment on attachment 8834768 [details] [diff] [review]
border-collapse

Reverts to pre-49 behavior in some cases to avoid a regression.
Attachment #8834768 - Flags: approval-mozilla-beta?
Attachment #8834768 - Flags: approval-mozilla-beta+
Attachment #8834768 - Flags: approval-mozilla-aurora?
Attachment #8834768 - Flags: approval-mozilla-aurora+
Needs a rebased patch for Beta uplift.
Flags: needinfo?(matt.woodrow)
Flags: needinfo?(matt.woodrow)
You need to log in before you can comment on or make changes to this bug.