Closed Bug 248239 Opened 20 years ago Closed 12 years ago

borders and padding on the top and bottom of table cells reduce the height

Categories

(Core :: Layout: Tables, defect)

Product:
Core
Component:
Layout: Tables
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla16

People

(Reporter: TailsTheKitsune, Assigned: tal.aloni.il)

References

Details

(Keywords: dev-doc-needed, testcase)

Attachments

(10 files, 11 obsolete files)

testcase
580 bytes, text/html; charset=UTF-8
Details
testcase
909 bytes, text/html
Details
padding testcase
599 bytes, text/html
Details
Padding testcase #2
602 bytes, text/html
Details
Research on table, row, cell and height
7.48 KB, text/html
Details
Caption Test Case 1
876 bytes, text/html
Details
Caption Test Case 1, in quirks mode
774 bytes, text/html
Details
test for caption/cell width/height, standards mode
801 bytes, text/html
Details
test for caption/cell width/height, quirks mode
785 bytes, text/html
Details
Table Cell Height Patch v8
4.72 KB, patch
dbaron
: review+
Details | Diff | Splinter Review
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a2) Gecko/20040621 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a2) Gecko/20040621 If you create a td that is styled to be 16x16 and give it a 4px border-right, the td will be 20px in width (try this with a background-color to verify). Same goes for border-left. However, if you apply a 4px border-top or border-bottom, the td will still be 16px in height. The 4px border will overlap in the 16px height rather than add the 4px to it. Reproducible: Always Steps to Reproduce: 1. 2. 3. Actual Results: border-top and border-bottom border lengths don't add the necessary height. Expected Results: Giving a 16x16 td a 4px border-top / border-bottom should add 4px to the height in the same manner that border-left /border-right add to the width. Seems to be a flaw in the CSS Box Model. If you specify both width and height length and color through the shorthand border: property, it is handled properly.
The <td> created in this attached page is supposed to be square but the border types aren't treated the same.
WFM 20040622 PC/WinXP
Assignee: general → dbaron
Component: Browser-General → Style System (CSS)
QA Contact: general → ian
Attachment #151498 - Attachment mime type: text/html → application/xhtml+xml
Comment on attachment 151498 [details] Shows an example of how border-top / bottom don't match border-left / right's behavior <?xml version="1.0" encoding="ISO-8859-1"?> <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd"> <html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en"> <head> <title> New Document </title> <style type="text/css"> td{background-color:red;width:16px;height:16px} </style> </head> <body> <table cellspacing="0" cellpadding="0"> <tr><td style="border-right:4px outset blue;border-top:4px outset green"></td></tr> </table> </body> </html>
Comment on attachment 151498 [details] Shows an example of how border-top / bottom don't match border-left / right's behavior Uploading a new attachment (the current one had all borders. I meant to undo these changes before uploading.
Could you actually attach the testcase that shows the problem?
Attached file TESTCASE (obsolete) —
My original attachment was intended to be a textcase (before I know what a textcase was). I tried editing it since I applied the values to all borders, but that resulted in making a reply to the thread with the text of the attachment. This time I'll create a new attachment from scratch. This will be my first testcase and I don't know if I'm doing it right (all I know is that Keywords aren't in my permissions yet, so I'll put TESTCASE as the comment). In the attachment, there is a table with 0 for cellspacing and cellpadding. There is no border specified for the table. The td tag has no content. Through CSS, the td is told to be 32x32 and colored gray. Also through CSS, it has an 8px solid red top and right border. The result is a 40x32 cell. 8px of the 40px (this is expected). The height is 32px instead of the expected 40px. Of that 32px, 8px is the red border and 24px is the gray. The height of the top border was not added to the height, but it has overwritten the top 8px of the existing 32px .
Attachment #151498 - Attachment is obsolete: true
Attached file testcase —
That certainly shows the bug, which is what we need. But it's even better to have testcases like this, where it's immediately obvious whether the bug is still present.
Attachment #152897 - Attachment is obsolete: true
Assignee: dbaron → nobody
Status: UNCONFIRMED → NEW
Component: Style System (CSS) → Layout: Tables
Ever confirmed: true
QA Contact: ian → core.layout.tables
Summary: CSS border-left / right px add to width but top / bottom do not.add to height → bottom and top borders on table cells reduce the height
Looks like making table cells have content-box box-sizing wasn't quite successful....
I also found this bug. I noticed that attached testcase IE 6 and Mozilla 1.7.3 render differently. Mozilla counts top and bottom border 50% into height (so that box height is 40px) and IE does not (it draws 10px border around 30px high content space what makes box height 50px). I thought - IE is wrong. But it is not. From http://www.w3.org/TR/REC-CSS2/images/tbl-spacing.gif it is clearly visible that border is not counted into cell width (so it is probably the same with height). I added this attachment because it works in IE and does not work in Mozilla.
Attached file testcase —
These boxes should be the same height.
*** Bug 351489 has been marked as a duplicate of this bug. ***
Attached file padding testcase —
From Bug 351489, this also happens with padding. This is attachment 152910 [details] with the border changed to padding.
Summary: bottom and top borders on table cells reduce the height → borders and padding on the top and bottom of table cells reduce the height
Still broken in Camino 2007102517 (1.5.3), Firefox (Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-GB; rv:1.8.1.9) Gecko/20071025 Firefox/2.0.0.9)
Flags: blocking1.9?
Flags: blocking1.9? → blocking1.9-
Attached file Padding testcase #2 —
There's a bug in Attachment 236904 [details] (padding testcase): Firefox seems to assume a default padding of 1px (or so), which makes the boxes 34 pixels wide. Of course, adjusting padding-top from 1px to 12px is only an increase of 11px. Fixed version sets the default padding (and border) to 0px (I suspect the standard doesn't specify a default padding or border).
This goes wrong at http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/tables/nsTableRowFrame.cpp&rev=3.406&mark=637#617 where the style height is passed, while border and padding are ignored. The current behavior is basically corresponding to IE rendering in quirks mode. http://developer.mozilla.org/en/docs/CSS:-moz-box-sizing (see IE7 for reference).
Keywords: testcase
(In reply to comment #15) > The current behavior is basically corresponding to IE rendering in quirks mode. In IEs Standards Mode, the cell height is calculated using the content-box model. The same is true for Safari and seems to be in line with CSS 2.1; Our current behavior should probably be quirks only. An alternate would be to define this on the UA style sheet level, so authors can overwrite the current layout which is not possible at the moment.
Flags: wanted1.9.2?
OS: Windows XP → All
Hardware: x86 → All
Daniel, a set of reftests and clear description how IE8, Safari behaves + testing of quirks mode /standards mode + rowspans + interference of row height and cell height + testing of the box-sizing https://developer.mozilla.org/en/CSS/box-sizing in different browsers for table cells and rows and the code changes should be fairly easy.
Bernd, this is only partially what you asked for, but maybe it's useful. Unfortunately, I won't have the time to complete this in the near future.
Daniel, its indeed useful. Don't feel pressured, this is table land where bugs have time to mature ;-) . side note: 1. I am very grateful for the work that you are doing. 2. QA with the ability to read a spec together with a working build environment is a dream combination (read the early bugs of bz and dbaron and you will recognize the path)
Flags: wanted1.9.2?
Flags: wanted1.9.2-
Flags: blocking1.9.2-
Attached patch Table Height Bugfix Patch (obsolete) — — Splinter Review
This patch fixes the incompatiblity problem with the CSS standard. The height calculation now uses the '-moz-box-sizing' property to determine which box-sizing model to use, and we use the content-box model if none is specified, as mandated by CSS specs. p.s. The file titled 'padding testcase' is an invalid test case, and there should be a difference of 1px in height between the cells (because by default firefox uses 1px padding for each table cell)
Comment on attachment 631395 [details] [diff] [review] Table Height Bugfix Patch >--- C:/FirefoxOriginalLayout/layout/tables/nsTableRowFrame.cpp Fri Jun 01 14:04:13 2012 >+++ C:/firefox/mozilla-release/layout/tables/nsTableRowFrame.cpp Fri Jun 08 15:46:26 2012 >@@ -636,11 +636,24 @@ > PRInt32 rowSpan = tableFrame->GetEffectiveRowSpan(*aCellFrame); > > switch (position->mHeight.GetUnit()) { >- case eStyleUnit_Coord: >+ case eStyleUnit_Coord: { >+ nscoord outsideBoxSizing = 0; >+ switch (position->mBoxSizing) { >+ case NS_STYLE_BOX_SIZING_CONTENT: >+ outsideBoxSizing = aCellFrame->GetUsedBorderAndPadding().TopBottom(); >+ break; >+ case NS_STYLE_BOX_SIZING_PADDING: >+ outsideBoxSizing = aCellFrame->GetUsedBorder().TopBottom(); >+ break; >+ } Some compilers are going to give a warning if you don't have a default: case in this switch, which is probably a good idea for readability since there is in fact another case. > specifiedHeight = position->mHeight.GetCoordValue(); >+ specifiedHeight += outsideBoxSizing; I think we should probably be making this case only for standards mode, not for quirks mode. Also, you should just change the previous line and use + rather than using +=. > if (1 == rowSpan) > SetFixedHeight(specifiedHeight); > break; >+ } Use spaces here rather than a tab. Do we need to make any similar changes for widths?
Also, you should really do development against mozilla-central, not mozilla-release (if the name of your directory indicates what repository you're working against).
Thanks David, You are correct that we should making this case only for standards mode, not for quirks mode, fixed that in v2. > Do we need to make any similar changes for widths? My tests suggests that width is already properly set for <td> elements. (i.e. set based on the content-box model), it would be nice to have support for 'border-box' and 'padding-box' for the width as well, but it is a different matter than fixing this CSS violation bug. Tal
Attachment #631395 - Attachment is obsolete: true
Comment on attachment 631434 [details] [diff] [review] Table Height Height Bugfix Patch v2 Perhaps a better way to make this apply to standards mode only would be to put td { -moz-box-sizing: border-box } in (the existing rule with selector "td") in layout/style/html.css. (Much like the rule that's there for table.)
Attachment #631434 - Flags: review?(dbaron)
(Yes, I should have realized that the last time; sorry.)
David, Unless I'm mistaken, 'layout/style/html.css' will apply to standards mode as well, and will change the default from the mandated 'content-box' to 'border-box'. It is my opinion that we should always use 'content-box' in the standards mode.
David, I believe you were referring to 'quirk.css', which is a good idea, here is an updated patch. I also moved two occurences of '-moz-box-sizing: border-box' from 'html.css' to 'quirk.css' to comply with CSS specs.
Attachment #631434 - Attachment is obsolete: true
Attachment #631434 - Flags: review?(dbaron)
Yes, I did mean quirk.css, sorry. But let's do one thing at a time -- there's research here saying that based on what other browsers do, we can make the change for table cells. I'd want to see similar testing of other browsers for the other changes, and they'd probably belong in other bugs.
Attached file Caption Test Case 1 —
Attached file Table Test Case 1 (obsolete) —
David, my standpoint is that we should never violate the CSS specs, regardless of what other browsers do, otherwise it's slippery slope. I added two additional test cases for the two other violations I've suggested to fix: 1. Table caption test case: All other major browsers do not violate the CSS specs (i.e. use the content-box model): - Opera 11.64 - Google Chrome 19.0.1084.52 m - IE8 - Safari 5.1.7 2. Table test case: All of the other major browsers DO violate the CSS specs, however, the CSS specs ( http://www.w3.org/TR/CSS2/tables.html#separated-borders ) are crystal clear about what's right, and we must follow the specs.
Attached patch Table Cell Height Patch v4 (obsolete) — — Splinter Review
Well, never mind about the Table Test Case, The specs are not on par with HTML and XHTML1, and this issue does not belong to the current discussion. Attached an updated patch, I'm still including the fix for the table caption.
Attachment #631619 - Attachment is obsolete: true
Attachment #631631 - Attachment is obsolete: true
Attachment #631636 - Attachment is patch: true
Attachment #631636 - Attachment description: Table Cell Height Patch → Table Cell Height Patch v4
Attachment #631630 - Attachment mime type: text/plain → text/html
I just now ran the reftests, here is my favorite quote, taken from 'bc_borderoffset1_ref.html': <!-- the height for table cells includes the border --> :facepalm: needless to say, my patch did not pass several tests.
If the tests are not correct fix them, they are not carved in stone
(In reply to Tal Aloni from comment #32) > 2. Table test case: > All of the other major browsers DO violate the CSS specs, however, the CSS > specs ( http://www.w3.org/TR/CSS2/tables.html#separated-borders ) are > crystal clear about what's right, and we must follow the specs. In that case, we should get the specs fixed. That's much simpler than getting all browsers to change and has the advantage that it won't break content already on the Web that depends on the current behavior.
David, I believe the table test issue is going to be resolved with CSS3's box-sizing, with currently accepted behavior going to be the default using User Agent style-sheet, like we already do ATM with -moz-box-sizing p.s. I will post fixed reftests as suggested
Attachment #631636 - Attachment is obsolete: true
Attachment #631636 - Flags: review?(dbaron)
Attachment #633497 - Flags: review?(dbaron)
Comment on attachment 633497 [details] [diff] [review] Table Cell Height Patch v4 (With updated reftests) There's no basis for moving the caption box-sizing declaration to quirks.css. It should just be removed. As far as I can tell, no other browsers (tested Chrome, Opera, IE10) have box-sizing: border-box for captions in either standards mode or quirks mode. We shouldn't introduce a new quirks/standards difference if there's no need to do so. Therefore, you should remove these added lines in quirks.css: >+caption { >+ -moz-box-sizing: border-box; >+} >+ and fix any tests as necessary. You should also add a test to test that captions use content-box sizing, since there appears to be no test for that. r=dbaron with that fixed, though if the test changes are nontrivial I'd like to look at them
Attachment #633497 - Flags: review?(dbaron) → review+
Comment on attachment 633497 [details] [diff] [review] Table Cell Height Patch v4 (With updated reftests) Actually, there's a bit more testing that needs to happen here. In particular, you need to test width behavior for both table cells and table captions (in both standards mode and quirks mode). Could you attach additional testcases to the bug for those cases? I fear that this patch may be making table cell width behavior more quirky than it was before, though I could be wrong. (Changing marking to review- since I want to look at this again once you've responded to this issue; that doesn't mean I won't grant review+ on the next patch.)
Attachment #633497 - Flags: review+ → review-
Attached patch Table Cell Height Patch v5 (obsolete) — — Splinter Review
David, You are correct to point out that the caption box-sizing declaration should just be removed. I have removed it, there is no reftest regression as a result. Other browsers (Chrome, Safari, Opera) use the content-box for both quirks and standards mode when it comes to table caption. As I mentioned above, width is already properly set for <td> elements (content box), this patch does not change that. While it's true that it's somewhat confusing that -moz-box-sizing will only be applied to table cell height after this patch is committed, in my opinion it's not "more quirky than it was before". I did wrote a separate patch that apply -moz-box-sizing to table width, but I preferred to seperate the two, as this one addresses CSS violation and the other is merely a feature.
Attachment #633497 - Attachment is obsolete: true
Attachment #636514 - Flags: review?(dbaron)
in the above paragraph: "table width" should be "table cell width".
(In reply to Tal Aloni from comment #42) > I did wrote a separate patch that apply -moz-box-sizing to table cell width, but > I preferred to seperate the two, as this one addresses CSS violation and the > other is merely a feature. But if you do that, it will suddenly start applying the declaration that's in quirks.css to table cell widths too, which isn't what we want. I think we might need to apply -moz-box-sizing separately for widths vs. heights. And at the very least we'd need to do that before taking both this fix and the other one you describe. But I suppose we could probably take one of them, as long as we have tests to verify that we don't take both.
(In reply to David Baron [:dbaron] from comment #44) > But if you do that, it will suddenly start applying the declaration that's > in quirks.css to table cell widths too, which isn't what we want. It's exactly what we want, in quirks mode, table cell box-sizing should be border-box for both width and height, this is the old html specification, this is the solution suggested by CSS3, and this is how other browsers behave.
(In reply to Tal Aloni from comment #45) > It's exactly what we want, > in quirks mode, table cell box-sizing should be border-box for both width > and height, this is the old html specification, this is the solution > suggested by CSS3, and this is how other browsers behave. OK, sounds good, then. I didn't see any tests for that here, though.
(In reply to David Baron [:dbaron] from comment #46) > I didn't see any tests for that here, though. This is because this patch does not affect <td> width, not directly, and not indirectly (because ATM <td> height was and still is content-box, regardless of -moz-box-sizing) You're going to see the relevant tests with the other patch, I do not want to create a mess and mix the two separate issues.
in the paragraph above: ..<td> *width* was and still is...
(In reply to Tal Aloni from comment #45) > (In reply to David Baron [:dbaron] from comment #44) > > But if you do that, it will suddenly start applying the declaration that's > > in quirks.css to table cell widths too, which isn't what we want. > It's exactly what we want, > in quirks mode, table cell box-sizing should be border-box for both width > and height, this is the old html specification, this is the solution > suggested by CSS3, and this is how other browsers behave. standards http://software.hixie.ch/utilities/js/live-dom-viewer/saved/1617 quirks http://software.hixie.ch/utilities/js/live-dom-viewer/saved/1618 Opera and current Firefox have border-box for height but not for width, in both modes. Chrome and IE8 have border-box for height but not for width in quirks mode, and content-box for both in standards mode.
Thanks for pointing that out Simon, regarding cell width in quirks mode, I mixed up between table and the table cell, Sorry and thanks for the correction! Both Opera and Firefox should use content-box by default for table cell height in standards mode, any other option is a CSS violation. Quirks mode: ------------ You are correct to suggest that in quirks mode some browsers use different box-sizing method for width and height, IE6 / IE8 / Chrome / Safari use *padding-box* for height and content-box for width. I guess we should thank Microsoft for that.
Sorry, not *padding-box* but border-box.
(In reply to David Baron [:dbaron] from comment #44) > I think we might need to apply -moz-box-sizing separately for widths vs. > heights. I'm not sure what you mean by that, it would be great If I could differentiate between the two: 1. -moz-box-sizing was set to content-box (default) 2. -moz-box-sizing was not set at all. any idea how to do that?
(In reply to Tal Aloni from comment #50) > Thanks for pointing that out Simon, regarding cell width in quirks mode, I > mixed up between table and the table cell, Sorry and thanks for the > correction! > > Both Opera and Firefox should use content-box by default for table cell > height in standards mode, any other option is a CSS violation. Well we could get the CSS spec changed, right? Tables in CSS are pretty underdefined anyway. Is it good to have a difference between quirks and standards here? Are there any Web pages that rely on the IE/Chrome behavior in standards mode? > Quirks mode: > ------------ > You are correct to suggest that in quirks mode some browsers use different > box-sizing method for width and height, IE6 / IE8 / Chrome / Safari use > *padding-box* for height and content-box for width. I guess we should thank > Microsoft for that. If we want to have a difference between quirks and standards mode here, should the quirk apply to only HTML <td> and <th> elements or all CSS 'table-cell's? Only <td> and <th> if they're 'table-cell'? I ask because I'd like to spec this in http://simon.html5.org/specs/quirks-mode if we go this route. Cheers.
(In reply to Simon Pieters from comment #53) > Well we could get the CSS spec changed, right? Alternatively, if it should only apply to HTML <td> and <th> (in all modes), it could be added to the Rendering section of the HTML spec.
(In reply to Simon Pieters from comment #53) > Well we could get the CSS spec changed, right? Simon, It would be better it you would simply make sure Opera match the standard, as I try to do here for firefox. I strongly believe that this is the right thing to do, Thanks! > If we want to have a difference between quirks and standards mode here, > should the quirk apply to only HTML <td> and <th> elements or all CSS > 'table-cell's? Only <td> and <th> if they're 'table-cell'? It only really matters for HTML <td> and <th> elements. Earlier versions of IE do not support 'display:table-cell', so it's not very common with pages that have no DOCTYPE. > (I filed https://www.w3.org/Bugs/Public/show_bug.cgi?id=17606 about > border-box on <table>) Thanks, AFAIK this is already planned for CSS3.
Attached patch Table Cell Height Patch v6 (obsolete) — — Splinter Review
Note: This patch is equivalent to v5, the only difference is that now border-box is hard-coded for quirks mode, instead of relying on quirk.css, This is because there is an historic anomaly in quirks mode: table cell width is content-box, but height is border-box. I made this change to prepare the ground for a patch that adds support for box-sizing on cell width.
Attachment #636514 - Attachment is obsolete: true
Attachment #636514 - Flags: review?(dbaron)
Attachment #636852 - Flags: review?(dbaron)
Comment on attachment 636852 [details] [diff] [review] Table Cell Height Patch v6 >+ // In quirks mode, table cell width should be content-box, but height should be border-box >+ // Because of this historic anomaly, we do not use quirk.css Could you: * wrap this comment before the 80th column (and use a . at the end of the first sentence) * add to the second sentence something like "since we can't specify one value of box-sizing for width and another for height" r=dbaron with that Also, the patch should have appropriate user and commit message data inside the patch for checkin. I'd suggest the commit message (all as one line): Change standards mode height calculations for table cells to use content-box sizing rather than border-box sizing by default (and to honor -moz-box-sizing, which we do not do in quirks mode). Also remove -moz-box-sizing: border-box from default style for caption element (all modes). (Bug 248239) r=dbaron
Attachment #636852 - Flags: review?(dbaron) → review+
Attached patch Table Cell Height Patch v7 (obsolete) — — Splinter Review
Attachment #636852 - Attachment is obsolete: true
Attachment #636886 - Flags: review?(dbaron)
Thanks for your help David, Since this is a bugfix, I suggest to fast-track this, and push it to central, aurora and beta branch.
Pushed to mozilla-inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/1e2b9cdc486b from which it will get merged to mozilla-central (unless it gets backed out for test failures). (In reply to Tal Aloni from comment #62) > Since this is a bugfix, I suggest to fast-track this, and push it to > central, aurora and beta branch. This should not land on aurora or beta. It's a fix for a longstanding bug, and it has some risk that it will break browser-specific content (for which we should at least give sites the full aurora/beta cycle), or that it actually does something that's not compatible with other browsers and breaks cross-browser content (for which we use the time on aurora or beta to get bug reports).
Assignee: nobody → tal.aloni.il
Target Milestone: --- → mozilla16
Comment on attachment 636886 [details] [diff] [review] Table Cell Height Patch v7 Also, you should generate diffs using "hg diff" (or by pulling patches out of a mercurial queue), not with tools that generate diffs relative to a different root (i.e., inside mozilla/layout/, which hg diff would not generate).
Attachment #636886 - Flags: review?(dbaron) → review+
Backed out of mozilla-inbound due to: 1063 ERROR TEST-UNEXPECTED-FAIL | /tests/MochiKit-1.4.2/tests/test_MochiKit-Style.html | default left table cell content h ok - got 34, expected 30 1065 ERROR TEST-UNEXPECTED-FAIL | /tests/MochiKit-1.4.2/tests/test_MochiKit-Style.html | default middle table cell content h ok - got 34, expected 30 1067 ERROR TEST-UNEXPECTED-FAIL | /tests/MochiKit-1.4.2/tests/test_MochiKit-Style.html | default right table cell content h ok - got 34, expected 30 1069 ERROR TEST-UNEXPECTED-FAIL | /tests/MochiKit-1.4.2/tests/test_MochiKit-Style.html | collapsed left table cell content h ok - got 32, expected 30 1071 ERROR TEST-UNEXPECTED-FAIL | /tests/MochiKit-1.4.2/tests/test_MochiKit-Style.html | collapsed middle table cell content h ok - got 32, expected 30 1073 ERROR TEST-UNEXPECTED-FAIL | /tests/MochiKit-1.4.2/tests/test_MochiKit-Style.html | collapsed right table cell content h ok - got 32, expected 30
Sorry about that, I thought that running 'make reftest' covers all tests, I'll review the Mochitests.
You can run just that one mochitest relatively easily, by setting the TEST_PATH environment variable. The question is whether the mochitest in question is something that passes in other browsers as well. Since it's part of the MochiKit tests, I'm worried that it may well be.
David, The Mochitest have a specific check for user agent in getElementDimensions, and for Gecko (and Opera) it compensates for the old (incorrect) behavior, So the Mochitest javascript library has to be updated (this odd quirk has to be removed), I'll make some more tests and post an updated patch.
(In reply to Tal Aloni from comment #56) > It only really matters for HTML <td> and <th> elements. http://software.hixie.ch/utilities/js/live-dom-viewer/saved/1630 http://software.hixie.ch/utilities/js/live-dom-viewer/saved/1631 AFAICT Opera and Firefox apply the quirk to display:table-cell elements but not to <td> and <th> elements that are display:block. (WebKit doesn't seem to support display:block on <td> in quirks mode.) So I guess I'll spec that.
Removed the bug workaround from the MochiKit library.
Attachment #636886 - Attachment is obsolete: true
Attachment #637085 - Flags: review?(dbaron)
Comment on attachment 637085 [details] [diff] [review] Table Cell Height Patch v8 r=dbaron, but could you get in touch with the authors of MachiKit and let them know they're going to need to change their version-sniffing here.
Attachment #637085 - Flags: review?(dbaron) → review+
Sure thing David, I've sent an email to Bob Ippolito, the maintainer.
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/4dd209b82d90 Thanks for the patch, Tal! To make life easier for those checking in on your behalf, please follow the directions at the link below for any future patches you submit. Thanks! https://developer.mozilla.org/en/Creating_a_patch_that_can_be_checked_in
Flags: wanted1.9.2-
Flags: in-testsuite+
Flags: blocking1.9.2-
Flags: blocking1.9-
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4dd209b82d90
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Depends on: 770014
Depends on: 770045
Depends on: 770106
No longer depends on: 770106
I wrote down the newly introduced quirks/standards mode difference. Please have a look and correct if something is wrong or my wording is ugly. https://developer.mozilla.org/en/Mozilla_Quirks_Mode_Behavior#Tables
j.j. you wrote "...remains as quirk for compatibility with other browsers", I think a more accurate description would be: "...remains as quirk for compatibility with *older* browsers", (which actually refers to old versions of IE) Thanks, Tal
(In reply to Tal Aloni from comment #78) > j.j. > you wrote "...remains as quirk for compatibility with other browsers", > I think a more accurate description would be: > "...remains as quirk for compatibility with *older* browsers" No, current WebKit and IE versions have this behaviour in quirks mode and will most likely never change it in future. But I will change to ...remains in quirks mode for compatibility with other browsers quirks mode behaviour
That's even more accurate, thanks!
Blocks: 338554
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: