table border drawn where it shouldn't, for rowspanning cell with "border-collapse: collapse" on table
Categories
(Core :: Layout: Tables, defect)
Tracking
()
People
(Reporter: blakewolf, Unassigned)
References
(Regression)
Details
(Keywords: regression)
Attachments
(4 files)
Steps to reproduce:
Screenshot of incorrect rendering attached.
IE, Chrome, and Edge, all render this as expected
Mozregression pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8991d660f20e3eea652e060c30e17670b45a9257&tochange=8b245cc1086f912f84b54a6af13f015404af8e14
Last Good: 2018-11-20
First Bad: 2018-11-21
Comment 2•3 years ago
|
||
The Bugbug bot thinks this bug should belong to the 'Core::DOM: Core & HTML' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.
Comment 3•3 years ago
|
||
Further narrowed regression window:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8b1b35582dd7f986619174828bae634511718de9&tochange=7488645b27ac9b273d6785db04204684673b3657
Updated•3 years ago
|
Comment 4•3 years ago
|
||
Set release status flags based on info from the regressing bug 1485179
Comment 5•3 years ago
|
||
I'll take a look here. Funny that a fix for a build warning introduced a rendering bug...
Comment 6•3 years ago
|
||
emilio suspects this might be related to this snippet added in the regressing bug:
https://searchfox.org/mozilla-central/rev/e66593593f3b356901011ea0fcdf9979728e9ae8/layout/tables/nsTableFrame.cpp#4708-4710
Updated•3 years ago
|
Updated•3 years ago
|
Comment 7•3 years ago
|
||
Here's a reduced testcase.
EXPECTED RESULTS: no border underneath the "no border" cell
ACTUAL RESULTS: There's a bottom-border-segment that juts out to the right, underneath the other text.
Current Nightly gives ACTUAL RESULTS.
Chrome and Firefox Nightly 2018-11-20 give EXPECTED RESULTS.
Interestingly: if I remove the orange border-right segment (e.g. by removing the with-border-right
class in devtools), then pre-regression Firefox Nightly 2018-11-20 changes its rendering and starts matching current Nightly, i.e. drawing the extended blue line underneath the "no border" text.
(Chrome does not do that.)
Comment 8•3 years ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #7)
Interestingly: if I remove the orange border-right segment (e.g. by removing the
with-border-right
class in devtools), then pre-regression Firefox Nightly 2018-11-20 changes its rendering and starts matching current Nightly, i.e. drawing the extended blue line underneath the "no border" text.
(Chrome does not do that.)
Here's a testcase where I've made that change. For this one, our behavior hasn't changed (but differs from Chrome) at least as far back as for as far back as Firefox 4 prereleases, 2010-11-20 (I can't test builds much older than that; they startup-crash on my system, presumably due to a library mismatch or something).
So: it seems to be the case that there's a longstanding interop issue in the neighborhood here (the rendering difference demonstrated by this testcase).
Nonetheless, it does seem like it was unintentional that the regressing patch should have had any impact on behavior. Still TBD whether that was a case where we inadvertently cleaned up something that was random/broken, vs. if we inadvertently changed away from our intended behavior...
Updated•3 years ago
|
Comment 9•3 years ago
|
||
For what it's worth, I tried all of the major current-and-historical non-Blink/non-Gecko engines[1] on both attached testcases here, and they all agree with Chromium on these attached testcases. So, that adds credence to the theory that Firefox's behavior here is a bug, or at least that the web likely depends on the consensus non-Firefox behavior here.
[1] Specifically: I tested Opera 12.16, IE 11, Edge 18 (EdgeHTML), and Safari 15.1, all via BrowserStack
Updated•3 years ago
|
Comment 10•3 years ago
|
||
I suspect this (testcase 2 at least) is a version of ancient bug 332740, specifically this seems like the "Bleeding borders" mentioned in attachment 348003 [details] (the "Another Test case" attachment on that bug).
Comment 11•3 years ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #6)
emilio suspects this might be related to this snippet added in the regressing bug:
https://searchfox.org/mozilla-central/rev/e66593593f3b356901011ea0fcdf9979728e9ae8/layout/tables/nsTableFrame.cpp#4708-4710
(nope, this turns out not to have been correct; I tried removing the & 0x7
bit-masking there, and it didn't change our rendering of the attached testcases.)
Updated•3 years ago
|
Comment 12•3 years ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #11)
(In reply to Daniel Holbert [:dholbert] from comment #6)
emilio suspects this might be related to this snippet added in the regressing bug:
https://searchfox.org/mozilla-central/rev/e66593593f3b356901011ea0fcdf9979728e9ae8/layout/tables/nsTableFrame.cpp#4708-4710(nope, this turns out not to have been correct; I tried removing the
& 0x7
bit-masking there, and it didn't change our rendering of the attached testcases.)
Did you also try restoring the original sizes of the *Elem and *Style bit fields?
Comment 13•3 years ago
|
||
(In reply to Chris Peterson [:cpeterson] from comment #12)
Did you also try restoring the original sizes of the *Elem and *Style bit fields?
Thanks -- I did not, but that indeed seems to be it!
If I just do s/ownerElem : 4;/ownerElem : 3;/
(restoring this one line, then we give EXPECTED RESULTS on testcase 1.
So: it seems likely that testcase 1 and testcase 2 are the same longstanding core problem, and we were just inadvertently working around the issue in certain cases like testcase 1, by virtue of having some bits improperly truncated (which we fixed in bug 1485179).
Comment 14•3 years ago
|
||
Here's a patch to demonstrate comment 13, using a 3-bit field instead of a 4-bit field. This "fixes" testcase 1 as well as the URL from comment 0 (by making us do improper/lossy type-conversions).
This isn't a patch we should actually land; I'm just posting it to illustrate how we got here via bug 1485179.
Updated•3 years ago
|
Comment 15•3 years ago
|
||
I'm going to dupe this against bug 332740, since it's the same root issue, which now (as of when bug 1485179 landed in 2018) applies to a few additional cases that we were previously papering-over by accident (with the accidental-paper-over being part of what bug 1485179 fixed).
The thing to do here at this point is to fix bug 332740, when we can prioritize that.
Updated•3 years ago
|
Description
•