Last Comment Bug 648531 - regression with table borders and the rules attribute
: regression with table borders and the rules attribute
Status: VERIFIED FIXED
:
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla6
Assigned To: fantasai
:
:
Mentors:
http://paulgrosso.name/firefox/table-...
Depends on:
Blocks: 43178
  Show dependency treegraph
 
Reported: 2011-04-08 07:50 PDT by Paul Grosso
Modified: 2011-07-20 06:50 PDT (History)
8 users (show)
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Paul's testcase (10.10 KB, text/html)
2011-04-08 10:03 PDT, Boris Zbarsky [:bz] (still a bit busy)
no flags Details
patch (4.00 KB, patch)
2011-04-11 23:02 PDT, fantasai
dbaron: review+
bzbarsky: review+
Details | Diff | Splinter Review

Description Paul Grosso 2011-04-08 07:50:59 PDT
User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0) Gecko/20100101 Firefox/4.0
Build Identifier: Firefox 4.0 ("up to date" as of 2011 April 8)

See http://paulgrosso.name/firefox/table-borders.htm for a self-explanatory example about a regression from Firefox 3.x (and divergence from Chrome and others and from my understanding of the HTML/CSS specs).

Due to this regression, there is no way to generate HTML/CSS that will work
properly on both Firefox 3.x and Firefox 4.

Reproducible: Always

Steps to Reproduce:
See http://paulgrosso.name/firefox/table-borders.htm
Actual Results:  
Chrome (10.0.648.204) gives what I believe are the correct results. Both Firefox 3.x and Firefox 4 give incorrect results, but the results are different and incompatible so that there is no way to code a page that works properly in all browsers.

Expected Results:  
What Chrome (10.0.648.204) shows.

While I'm leaving the severity Normal (it's hard to say a major feature is broken), this is a regression that will prevent people from upgrading to Firefox 4 in some cases.  My company makes HTML generating software, and we will not be able to provide Firefox 4 support (without breaking Firefox 3.x support) until this bug is fixed.
Comment 1 Boris Zbarsky [:bz] (still a bit busy) 2011-04-08 10:03:39 PDT
Created attachment 524644 [details]
Paul's testcase
Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2011-04-08 10:20:33 PDT
So the issue seems to be the handling of the rules="none" cases, right?

In Firefox 3.6 the "rules" attribute did weird stuff that's not describable in CSS.

The UA stylesheet in Firefox 4 has the following rules which are relevant here:
 
 table[frame] {
   border: thin hidden;
 }

 table[frame="box"], table[frame="border"] { border-style: outset; } 

 table[rules]:not([rules="none"]):not([rules=""]) {
   border-collapse: collapse;
 }

 table[rules][rules="none"]  > tr > td,
 table[rules][rules="none"] > * > tr > td,
 table[rules][rules="none"] > tr > th,
 table[rules][rules="none"] > * > tr > th,
 table[rules][rules="none"] > td,
 table[rules][rules="none"] > th
 {
   border-width: thin;
   border-style: hidden;
 }

So in the rules="none" case we set all the cell borders as "hidden" and do NOT set the table to border-collapse:collapse, because that would override the border created on the table by the "frame" attribute.

But the attached testcase manually sets the table to border-collapse:collapse (which is something that is NOT desirable for rules="none"), at which point hidden borders will override any other borders they collapse with.

The same effect is happening in the case when the page sets the bottom and right borders only on the cells: the hidden top and right borders hide whtever borders they collapse with.

I suppose we could use "none" instead of "hidden" here; is there a reason we're using "hidden"?

This CSS was added in bug 43178, for what it's worth.

Paul, I suspect just removing your additional border-collapse rules and relying solely on the "frame", "border", and "rules" attributes will make things do the right thing for the cases those attributes handle.  For the cases they don't handle, you don't want to be doing them anyway and want to explicitly style things the way they want them styled...

In general, the question of what exactly those three attributes should do is still an open one; if you have concrete proposals please post then to either the WHATWG or W3C HTML working group list.
Comment 3 fantasai 2011-04-08 11:09:19 PDT
I believe the second rule could indeed be none, but the [frame] one must be hidden.
Comment 4 Boris Zbarsky [:bz] (still a bit busy) 2011-04-08 11:12:14 PDT
Yeah, the @rules rule is the one that matters here.  fantasai, do you want to take this bug?
Comment 5 Paul Grosso 2011-04-08 11:49:21 PDT
I just added a copy of the last CSS rule you show above to the .css referenced by my test case and changed hidden to none, and that appears to work (in Firefox 4, Firefox 3.x, Chrome and IE), so that's great news for me, because I do need to set border-collapse to get things to work as desired in the general case I need to handle.  

If you want to see the kinds of things I need to support, see http://paulgrosso.name/firefox/asdbsamp.htm 

With that added CSS rule, that file displays as desired in all the browsers I tested.

Thanks for the quick responses.
Comment 6 fantasai 2011-04-11 23:02:03 PDT
Created attachment 525313 [details] [diff] [review]
patch

Ta-da! One line html.css patch. Testcase included. And now I want to refactor the CSS here.
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2011-05-02 12:19:40 PDT
Comment on attachment 525313 [details] [diff] [review]
patch

r=me if David is ok with that.
Comment 8 David Baron :dbaron: ⌚️UTC-7 2011-05-11 01:20:41 PDT
Comment on attachment 525313 [details] [diff] [review]
patch

So it looks like this leads us to the pattern that we always use
'hidden' style for the frame/rules handling rules that apply to
table, and always use 'none' for those that apply to th/td?  I
suppose there's at least some logic in that, so r=dbaron, though I'm
a little uncomfortable with the remaining 'hidden's overriding
author-level rules when border-collapse is collapse.

Anyway, r=dbaron
Comment 9 Boris Zbarsky [:bz] (still a bit busy) 2011-05-11 08:32:00 PDT
fantasai, feel free to toss a checkin-needed on this bug if you don't want to deal with the tree.
Comment 10 Daniel Holbert [:dholbert] 2011-05-12 10:59:56 PDT
http://hg.mozilla.org/projects/cedar/rev/2dd8b7415752
Comment 11 Mounir Lamouri (:mounir) 2011-05-13 02:15:45 PDT
Pushed:
http://hg.mozilla.org/mozilla-central/rev/2dd8b7415752
Comment 12 Simona B [:simonab ] 2011-07-20 06:50:19 PDT
Mozilla/5.0 (Windows NT 5.1; rv:6.0) Gecko/20100101 Firefox/6.0

Verified issue on Firefox 6.0b2 using Paul's test case by comparing with other browsers. Tested on Win XP, Win 7, Mac OS X 10.6 and Ubuntu 11.04.

Setting resolution to VERIFIED FIXED.

Note You need to log in before you can comment on or make changes to this bug.