value of 'collapse' needs to be ignored for border-collapse: style property

VERIFIED FIXED in M18

Status

()

defect
P1
critical
VERIFIED FIXED
19 years ago
19 years ago

People

(Reporter: attinasi, Assigned: karnaze)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nsbeta3-] [rtm++] a=buster, r=dcone)

Attachments

(1 attachment)

Reporter

Description

19 years ago
We need to ignore 'collapse' for border-collapse on tables since the collapsed 
borders are rendered incorrectly and we cannot fix them in time for beta3. Once 
we get time to fix the collapsing border model we can pay attention to the 
collapse value again.
Reporter

Comment 1

19 years ago
This is kinda painful, but necessary for beta3.
Keywords: nsbeta3
Summary: value of collapse needs to be ignored for border-collapse: style property → value of 'collapse' needs to be ignored for border-collapse: style property
Whiteboard: [nsbeta3+]
Reporter

Updated

19 years ago
Target Milestone: --- → M18

Comment 2

19 years ago
And did we actually do this?
Assignee

Comment 3

19 years ago
It will probably be done when there is no chance that the new version of 
collapsing borders will not make it in. So, it hasn't been disabled yet.

Comment 4

19 years ago
*Sigh at extreme communication difficulties*

YES, the new border collapsing code won't make it in, as Marc said:
------- Additional Comments From Marc Attinasi 2000-08-18 12:53 -------

This is kinda painful, but necessary for beta3.


Things just keep slipping through our fingers, don't they.

Reporter

Comment 5

19 years ago
I think that this should be ignored by the css parser, that way if the DOM
accesses the style value it cannot get a value of 'collapse' when in fact we are
treating it like 'separate'. At the same time, we need to:
1] make the CSSParser ignore the border-collapse property
2] ensure that the default for border-collapse in the stylecontext is 'separate'
3] remove the border-collapse: separate from html.css
4] ASSERT in table code that the value for border-collapse is always separate

1 is trivial, 2 is already done, 3 is trivial, and 4 is trivial and optional

We could just change the table code to always use the separate border model
regardless of the value of the style property, but then the DOM and the display
will be out of synch. I think it is better to act like we just don't recognize
the property at all.

Chris, if this is indeed the path we want/need to go, send me this bug and I'll
take care of the style system changes. Also, if this is beta3+ then we need to
do it right away, I think.
You need to make sure the property name can't be changed through the CSSOM.

Comment 7

19 years ago
Not holding PR3 for this. Marking nsbeta3-. Please nominate for rtm if we really
need to fix this before shipping seamonkey.
Whiteboard: [nsbeta3+] → [nsbeta3-]
BTW, an alternative solution to this might be to change the part of the table
code where the border model is selected based on style information, which I
suspect might be a very local change.  It would avoid CSSOM complications.
Reporter

Comment 9

19 years ago
If we only change the table code then the CSSOM can be out of sync. For example,
if there is a stylesheet that has border-collapes:collapsed the CSSOM will
return that value, but the table will use separate borders. If we are willing to
live with this then, yes, it is a more localized and less risky change.
I think that's how we behave for all CSS props we don't support, e.g.,
text-shadow.
Since border-collapse only takes two values, then there is no forwards-
compatability issue as there is with unsupported values on 'display'. Thus
I think David is right, we don't really need to worry about either the
parser nor the DOM, just block out the collapsing border code in table layout.
Reporter

Comment 12

19 years ago
I understand about the lack of forward-compatibility issues, but are you saying
it is OK if the CSSOM reports that the border-collapse is 'collapsed' when
really it is 'separate'? Granted, it doesn't sound like the end of the world,
but clearly it is wrong. That said, it is obviously easier to just hard-wire the
table code to always use the separate border model regardless of the style
context value.
Assignee

Comment 13

19 years ago
The nasty part of turning off collapsing borders will be to deal with turning 
off the rules attribute, since rules imply the collapsing border model.
Marc: This is a non issue, as as David pointed out, we already do it for 
attributes/properties that we don't support such as textShadow/text-shadow.
Assignee

Updated

19 years ago
Blocks: 54124
Assignee

Comment 15

19 years ago
Adding [rtm need info] and marking this P1 since fixing it fixes bug 54124.
Severity: normal → critical
Status: NEW → ASSIGNED
Priority: P3 → P1
Whiteboard: [nsbeta3-] → [nsbeta3-] [rtm need info]
Assignee

Updated

19 years ago
Blocks: 33175
Assignee

Updated

19 years ago
Blocks: 25228
Assignee

Comment 16

19 years ago
Assignee

Comment 17

19 years ago
changing to rtm+
Whiteboard: [nsbeta3-] [rtm need info] → [nsbeta3-] [rtm+] a=buster, r=dcone
Assignee

Comment 18

19 years ago
rtm++ per discussion with PDT.
Whiteboard: [nsbeta3-] [rtm+] a=buster, r=dcone → [nsbeta3-] [rtm++] a=buster, r=dcone
Assignee

Comment 19

19 years ago
Fixed
Status: ASSIGNED → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → FIXED

Updated

19 years ago
Status: RESOLVED → VERIFIED

Comment 20

19 years ago
Using David Baron's testcase for collapsing table borders:

http://www.people.fas.harvard.edu/~dbaron/css/test/sec1706c

with 10/6 build on Win, Mac and Linux, verified bug fixed. 'border-collapse: 
collapse' is turned off. Will contact Karnaze regarding 'rules'

Comment 21

19 years ago
karnaze, you forgot to disable nsTableFrame::ProcessGroupRules()
rows=groups still affects the table

Comment 22

19 years ago
sorry, that should be rules=groups :)
You need to log in before you can comment on or make changes to this bug.