Closed Bug 1755218 Opened 3 years ago Closed 3 years ago

table border drawn where it shouldn't, for rowspanning cell with "border-collapse: collapse" on table

Categories

(Core :: Layout: Tables, defect)

65 Branch
defect

Tracking

()

RESOLVED DUPLICATE of bug 332740

People

(Reporter: blakewolf, Unassigned)

References

(Regression)

Details

(Keywords: regression)

Attachments

(4 files)

Attached image Firefox.PNG (deleted) —

Steps to reproduce:

Screenshot of incorrect rendering attached.

IE, Chrome, and Edge, all render this as expected

https://halftime.wiaawi.org/CustomApps/Tournaments/Brackets/HTML/2022_Basketball_Girls_Div1_Sec1_2.html

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.

Component: Untriaged → DOM: Core & HTML
Product: Firefox → Core
Has Regression Range: --- → yes
Has STR: --- → yes
Component: DOM: Core & HTML → Layout: Tables
Keywords: regression
Regressed by: 1485179
Status: UNCONFIRMED → NEW
Ever confirmed: true

Set release status flags based on info from the regressing bug 1485179

I'll take a look here. Funny that a fix for a build warning introduced a rendering bug...

Flags: needinfo?(dholbert)
Summary: Incorrect rendering on HTML document → table border drawn where it shoudln't, for rowspanning cell with "border-collapse: collapse" on table
Attached file testcase 1 (deleted) —

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.)

(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...

Attachment #9265422 - Attachment description: testcase 2 → testcase 2 (demonstrates a version of the issue that goes back ~forever, i.e. this part's not a regression)
Flags: needinfo?(dholbert)

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

Summary: table border drawn where it shoudln't, for rowspanning cell with "border-collapse: collapse" on table → table border drawn where it shouldn't, for rowspanning cell with "border-collapse: collapse" on table

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).

(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.)

Severity: -- → S3

(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?

https://searchfox.org/mozilla-central/diff/b4cb33090278291483b26fbe85bc9b1c7f9d1496/layout/tables/nsTableFrame.cpp#5308-5312

Flags: needinfo?(dholbert)

(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).

Flags: needinfo?(dholbert)

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.

Assignee: nobody → dholbert
Assignee: dholbert → nobody

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.

Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: