Bug 330964 - (row-column-spacing) Add support for mtable@rowspacing/columnspacing/framespacing attributes
 (row-column-spacing) Summary: Add support for mtable@rowspacing/columnspacing/framespacing attributes
 Status: RESOLVED FIXED dev-doc-complete Core Components MathML (show other bugs) Trunk All All P3 normal with 3 votes (vote) mozilla33 James Kitchener (:jkitch) 69409 731667 975935 1031934 1061027 mathml-2 mathml-in-mathjax 958947 Show dependency tree / graph

Reported: 2006-03-18 14:46 PST by David Harvey
Modified: 2014-09-13 13:24 PDT (History)
11 users (show)
ryanvm: in‑testsuite+
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Attachments
Testcase based on reporter's code (1.27 KB, application/xhtml+xml)
2006-06-19 16:10 PDT, Steuard Jensen
no flags Details
tablespacing.png (5.36 KB, image/png)
2014-02-17 05:24 PST, James Kitchener (:jkitch)
no flags Details
Part1 Add arguments to GetCellSpacing[XY] WIP (48.41 KB, patch)
2014-02-18 01:50 PST, James Kitchener (:jkitch)
no flags Details | Diff | Splinter Review
Part 2: MathML mtable changes (16.95 KB, patch)
2014-02-18 01:54 PST, James Kitchener (:jkitch)
no flags Details | Diff | Splinter Review
Part 3: tests WIP (24.79 KB, patch)
2014-02-18 02:01 PST, James Kitchener (:jkitch)
no flags Details | Diff | Splinter Review
Part 0: Fix to CalcUnpaginagedHeight (1.13 KB, patch)
2014-02-23 05:00 PST, James Kitchener (:jkitch)
roc: review+
Details | Diff | Splinter Review
Part1: Add arguments to GetCellSpacing[XY] (49.78 KB, patch)
2014-02-23 05:12 PST, James Kitchener (:jkitch)
no flags Details | Diff | Splinter Review
Difference between spacing types (23.94 KB, image/png)
2014-02-23 23:49 PST, James Kitchener (:jkitch)
no flags Details
MathML mtable changes (19.21 KB, patch)
2014-02-24 00:33 PST, James Kitchener (:jkitch)
fred.wang: review+
Details | Diff | Splinter Review
Part 4: tests (45.98 KB, patch)
2014-02-24 00:37 PST, James Kitchener (:jkitch)
fred.wang: review+
Details | Diff | Splinter Review
testcase for attachment 8377104 (802 bytes, text/html)
2014-02-24 00:43 PST, James Kitchener (:jkitch)
no flags Details
Part 2: MathML mtable changes (20.42 KB, patch)
2014-02-26 03:08 PST, James Kitchener (:jkitch)
no flags Details | Diff | Splinter Review
Part 3: tests (58.23 KB, patch)
2014-02-26 03:14 PST, James Kitchener (:jkitch)
no flags Details | Diff | Splinter Review
Part 2: MathML mtable changes (25.48 KB, patch)
2014-03-03 01:24 PST, James Kitchener (:jkitch)
no flags Details | Diff | Splinter Review
Part 3: tests (62.02 KB, patch)
2014-03-03 01:38 PST, James Kitchener (:jkitch)
no flags Details | Diff | Splinter Review
Part 2: MathML mtable changes (25.48 KB, patch)
2014-03-03 01:44 PST, James Kitchener (:jkitch)
no flags Details | Diff | Splinter Review
Part1: Add arguments to GetCellSpacing[XY] (49.79 KB, patch)
2014-03-03 01:48 PST, James Kitchener (:jkitch)
roc: review-
Details | Diff | Splinter Review
Part 2: MathML mtable changes (25.34 KB, patch)
2014-06-13 03:23 PDT, James Kitchener (:jkitch)
no flags Details | Diff | Splinter Review
Part 3: tests (61.79 KB, patch)
2014-06-13 03:24 PDT, James Kitchener (:jkitch)
no flags Details | Diff | Splinter Review
Part1: Add arguments to GetCellSpacing[XY] (50.22 KB, patch)
2014-06-13 06:58 PDT, James Kitchener (:jkitch)
roc: review+
Details | Diff | Splinter Review
Part1: Add arguments to GetCellSpacing[XY] (51.63 KB, patch)
2014-06-15 00:53 PDT, James Kitchener (:jkitch)
no flags Details | Diff | Splinter Review
Part 2: MathML mtable changes (26.42 KB, patch)
2014-06-15 01:07 PDT, James Kitchener (:jkitch)
fred.wang: review+
Details | Diff | Splinter Review
Part 3: tests (62.18 KB, patch)
2014-06-15 01:10 PDT, James Kitchener (:jkitch)
fred.wang: review+
Details | Diff | Splinter Review
Part 2: MathML mtable changes (26.48 KB, patch)
2014-06-15 07:23 PDT, James Kitchener (:jkitch)
no flags Details | Diff | Splinter Review
Part 4: Flag failing tests (1.58 KB, patch)
2014-06-17 05:56 PDT, James Kitchener (:jkitch)
karlt: review+
Details | Diff | Splinter Review
Part1: Add arguments to GetCellSpacing[XY] (51.70 KB, patch)
2014-06-18 06:47 PDT, James Kitchener (:jkitch)
no flags Details | Diff | Splinter Review
Part 3: tests (62.76 KB, patch)
2014-06-18 06:49 PDT, James Kitchener (:jkitch)
no flags Details | Diff | Splinter Review
Part 4: Flag failing tests (1.58 KB, patch)
2014-06-18 06:51 PDT, James Kitchener (:jkitch)
no flags Details | Diff | Splinter Review
Part 2: MathML mtable changes (26.48 KB, patch)
2014-06-18 06:57 PDT, James Kitchener (:jkitch)
no flags Details | Diff | Splinter Review

 David Harvey 2006-03-18 14:46:21 PST User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en) AppleWebKit/417.9 (KHTML, like Gecko) Safari/417.8 Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1 The "rowspacing" attribute for MathML tables is not supported. According to the MathML spec, this attribute controls the amount of space between rows in the table. Correct implementation would allow constructs like AMS-LaTeX's "\substack" command. For example, the following three chunks should render differently, but Gecko draws them the same way: == example 1 == x x == example 2 == x x == example 3 == x x Reproducible: Always Steps to Reproduce: 1. View a page with the above MathML Actual Results: Example 1 should have widely spaced rows; example 2 should tightly spaced rows; example 3 should be somewhere in between. Expected Results: All three tables have the same amount of inter-row space (I'm guessing 1ex). I've only tried this on Mac OS, haven't tried other platforms. Steuard Jensen 2006-06-19 16:10:56 PDT Created Testcase based on reporter's code rbs 2006-08-20 19:49:10 PDT The spec seems a bit vague as to the meaning of the rowspacing attribute (and its cousin -- columnspacing): "The rowspacing and columnspacing attributes specify how much space should be added between each row and column. However, spacing before the first row and after the last row (i.e. at the top and bottom of the table) is given by the second number in the value of the framespacing attribute, and spacing before the first column and after the last column (i.e. on the left and on the right of the table) is given by the first number in the value of the framespacing attribute." 1/ Case of a mathbackground set on a row, I have always understandood the MathML's rowspacing as HTML's cellpadding. Hence I would expect the extra space to go in the cell and thus get the specified mathbackground color. (Note that this is different from HTML's cellspacing -- which is outside the cell, and does not get the background-color of the cell). 2/ Case of a table with rowlines, consider this mtable: row1 ---- <---- solid rowline row2 has a "columnalign" attribute, private "-moz-math-columnalign" attributes are created on 's and the style is then given by the mathml.css sheet: http://mxr.mozilla.org/mozilla-central/source/layout/mathml/mathml.css#256 This works when we only have a small set of values, but not for attributes with a length value: http://mxr.mozilla.org/mozilla-central/source/layout/mathml/mathml.css#303 BTW, I don't like much this method because it adds unwanted attributes in the DOM (see bug 527201) and I suspect it is causing the layout issue of bug 491384. I guess the appropriate solution would be fixing bug 69409... Bernd 2010-08-12 22:31:07 PDT shouldn't rowspacing map to borderspacing.mY and columnspacing to borderspacing.mX like cellspacing http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/nsHTMLTableElement.cpp#1065 ? Frédéric Wang (:fredw) 2010-08-13 03:52:20 PDT mtable seems a bit different, because rowspacing can have several values and we also have framespacing. Here is how I understand the spec for a mtable of N rows, framespacing="X" and rowspacing="Y0, Y2, ..., Y_(n-1)": ^ | X V ----- ///// row0 ///// ----- ^ | Y_(0 mod n) V ----- ///// row1 ///// ----- ^ | Y_(1 mod n) V ----- ///// ... ///// ----- ^ | Y_(N-2 mod n) V ----- ///// row_(N-1) ///// ----- ^ | X V However, the spec does not provide any attribute "rowpadding" that would allow to describe the spacing inside the cell. I guess that's why RBS wanted instead to split the rowspacing half inside and half outside the cell... Frédéric Wang (:fredw) 2012-11-30 08:25:31 PST Mass change: setting priority to 3 for bugs preventing Gecko's Native MathML to be enabled by default in MathJax. Frédéric Wang (:fredw) 2014-01-10 06:15:16 PST Here is a more info for people wanted to work on this: == DESCRIPTION === The goal is to implement the rowspacing and columnspacing on the mtable element (perhaps framespacing too): http://www.w3.org/TR/MathML/chapter3.html#presm.mtable.attrs I asked precision on the Math WG mailing list some time ago: http://lists.w3.org/Archives/Public/www-math/2010Aug/0004.html The rowspacing attribute is a list of lengths "l_0 l_1 l_2 ...". If the table has N rows, then the spacing applies to the N-1 gaps: [ROW_0] ↕ l_0 [ROW_1] ↕ l_1 [ROW_2] ↕ l_2 ... [ROW_(N-1)] ↕ l_(N-1) [ROW_N] The note about "multiple values" say that if there are more gaps than supplied values, the last value is repeated as needed. If there are too many values supplied, the excess are ignored. The columspacing attribute is similar but applies to gaps between columns. In theory these attribute values can come from an mstyle ancestors (bug 768819) but we want to WONTFIX that one after bug 838509, so no need to worry about that. As said in comment 5, this seems to correspond to HTML attributes cellspacing: http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/HTMLTableElement.cpp#703 https://developer.mozilla.org/en-US/docs/Web/CSS/border-spacing except that the spacing can be different for each gap between column/row so it's a bit more complicated. Also, note that the border-spacing is used for the spacing around the table, so it might be relevant to consider the mtable@framespacing attribute as well: this one takes two length values and specifies the additional spacing added between the table and frame, if frame is not "none". The first value specifies the spacing on the right and left; the second value specifies the spacing above and below. === PROPOSAL == The work is very similar to what Quentin did in bug 731667. I see essentially three steps (so three separate patches): 1) Tests (see [review]). Some ideas: - rowspacing/columnspacing - single/multiple values - not enough values / too much values - invalid attributes - dynamic changes via Javascript - various values for spacing including 0 and default 2) Parsing of rowspacing/columnspacing See http://mxr.mozilla.org/mozilla-central/source/layout/mathml/nsMathMLmtableFrame.cpp and in particular MapAllAttributesIntoCSS where we create new FrameProperties. On the one hand, this can be easier because you only have to consider attributes from the (not or ) but on the other hand you have to store a list of nsCSSValues returned by nsMathMLElement::ParseNumericValue (http://mxr.mozilla.org/mozilla-central/source/content/mathml/content/src/nsMathMLElement.cpp), rather than just enumerate values like center/left/right, so the code should be adapted. 3) Rendering of rowspacing/columnspacing You'll have to generalize the layout/tables implementation from which nsMathMLmtableFrame is derived so that it is able to handle different different spacing for each gap between column/row. Perhaps one way would be to make nsTableFrame::GetCellSpacingX() and nsTableFrame::GetCellSpacingY() take an (optional?) argument indicating the gap index (perhaps using -1 and N for the spacing around the table, or something like that). Then every places where it is used in layout/tables/ should be modified to specify the gap index explicitly. nsTableFrame::GetCellSpacingX() and nsTableFrame::GetCellSpacingY() will just ignore that gap index and use the frame-spacing value, so the HTML implementation won't be affected. However nsMathMLmtableFrame will override this GetCellSpacingX/GetCellSpacingY to take into account the gap index and to return the value from FrameProperties, when specified. An alternative for 2) and 3): one could create new private CSS properties -moz-row-spacing / -moz-column-spacing that would handle a list of length and map the MathML attributes to these properties on the content/ side. Then modify layout/tables/ to consider these values in place of frame-spacing, when they are specified. James Kitchener (:jkitch) 2014-02-17 05:24:10 PST Created tablespacing.png The attached picture shows a 2x2 , with each containing a single . row/column/framespacing are all set to 0 (or 0 0). Document set to standards compliance mode. The blue dashed border indicates the element's outline, the red border that of the elements and the black border that of . I interpret the touching of the borders as indicating that the 0 spacing is rendered appropriately. What I haven't worked out is what is causing the spacing within the element within standards compliance mode. (It disappears when rendered in quirks mode). I've tried zeroing the margin and padding for everything in the page, but without success. Can you identify the cause of the whitespace? I presume it is something that needs to be taken into account, otherwise 0.5ex will be added to whatever rowspacing the user specifies. Frédéric Wang (:fredw) 2014-02-17 05:36:13 PST Do you have a testcase / patch that you used to take that screenshot? James Kitchener (:jkitch) 2014-02-18 01:50:09 PST Created [review] Part1 Add arguments to GetCellSpacing[XY] WIP Adds an row/column index argument to GetCellSpacing. The two argument version is a nicety that calculates the spacing of multiple columns. In the default tableFrame case it simply multiplies the answer out. These functions are to be overloaded by nsMathMLmtableFrame. I'm not yet certain of selecting the correct row/column in all cases, but if the code isn't exercised by MathML the choice is unimportant and won't change the outcome. I suppose I can only use the argument method on instances invoked by MathML tables and have the argument-less one assert in such situations. James Kitchener (:jkitch) 2014-02-18 01:54:29 PST Created [review] Part 2: MathML mtable changes As far as I can tell row/column/framespacing isn't exposed to anything except mtable, so performing the parsing within the class seems reasonable. It mostly works, but there are a few test failures still to resolve and more tests are needed. James Kitchener (:jkitch) 2014-02-18 02:01:21 PST Created [review] Part 3: tests WIP Not yet exhaustive tests are attached. corresponds to tablespacing-5.html although I doubt that particular test will be included as part of the final set. Tests to ensure that rowspacing="5ex" really creates a 5ex space are yet to be written. James Kitchener (:jkitch) 2014-02-23 05:00:40 PST Created [review] Part 0: Fix to CalcUnpaginagedHeight The purpose of CalcUnpaginagedHeight is to compute a height, which is presumably a vertical distance. Everything within the function deals with a vertical difference except the declaration of computedHeight, which seeks to multiply a number of rows (rowspan - 1) with the distance between columns (GetCellSpacingX()). This I believe to be a mistake. GetCellSpacingY() which returns the distance between two rows seems a more appropriate fit. James Kitchener (:jkitch) 2014-02-23 05:12:56 PST Created [review] Part1: Add arguments to GetCellSpacing[XY] The spacing between rows and clumns in MathML mtables is variable, so I would like to add an argument to GetCellSpacing[XY] to specify which row/column space to use. In ordinary tables the argument doesn't matter, so there shouldn't be any effect other than the asserts ensuring sensible values are passed in. These asserts are also helpful in checking whether the correct function is called in situations where the number of rows and columns differ. The two argument version of GetCellSpacing[XY] is a convenience to allow cumulative spacing to be determined by a single multiplication when index doesn't matter, rather than repeatedly adding the same operand. Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2014-02-23 13:20:11 PST Sorry but I have to ask: are these attributes really needed? Could authors just use CSS padding instead? James Kitchener (:jkitch) 2014-02-23 23:49:33 PST Created Difference between spacing types (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #16) > Sorry but I have to ask: are these attributes really needed? Could authors > just use CSS padding instead? My understanding is that two accomplish different things. In the attached diagram: left example sets mtd {padding:5px;} middle example sets mtable {border-spacing:5px} mtd{padding:0px;} right example sets rowspacing="5px 20px" columnspacing="5px 20px" framespacing="5px 5px" (using the patches from this bug) padding adds the space inside the table cells, which are then placed touching each other, resulting in a solid block of red. border-spacing leaves 5px of white between the cells, resulting in a windowpane effect. rowspacing/columnspacing is similar to the border-spacing example, except different spaces can be specified. Now I am not an expert in CSS, so the third example might still be replicable by other means but it may not be a simple process. Frédéric Wang (:fredw) 2014-02-24 00:20:53 PST One thing to consider is that in practice we are bit constrained by the current status of math: 1) LaTeX is the de facto standard. And in LaTeX we put all the table formatting as an argument of the environment command that generates the table. So it's a bit tedious for LaTeX-to-MathML converters to have to move all this formating to cell nodes, especially when they are fast stream filter like itex2MML where you are essentially handling a MathML string not a DOM tree. That's why LaTeX-to-MathML converters use columnspacing/rowspacing and the other alignment attributes already implemented. 2) MathML is still used in environment where CSS is not available. Or in polyfills like MathJax that don't work well with CSS. In particular, Wikipedia will rely on MathJax to do server-side LaTeX-to-MathML conversion and thus the MathML output served will contain columnspacing/rowspacing. Of course, MathML is designed with 1) and 2) in mind. That's a bit a pain that we have to modify so much of the table code to do that but I don't really see how we can do otherwise without duplicating the code into the MathML table module... James Kitchener (:jkitch) 2014-02-24 00:33:19 PST Created [review] MathML mtable changes Now passes tests. The old padding CSS has been stripped out, but I'm not sure if explicitly zeroing it is correct. A consequence of this patch is that css border-spacing on longer has any effect. James Kitchener (:jkitch) 2014-02-24 00:37:21 PST Created [review] Part 4: tests Tested: static and dynamic setting of attributes Default values Error handling (an unreadable value falls back to the default value) Pixel counting mochitest to ensure a spacing of 7px really is 7px. James Kitchener (:jkitch) 2014-02-24 00:43:46 PST Created testcase for Testcase for the screenshot posted earlier in . I've disabled padding and margin for everything, but cannot account for the gap between the border and the border. The attached patches ensure that rowspacing="0px" and rowspacing="10px" give a spacing beween elements of 0 and 10px respectively, but there is an additional 0.5ex in vertical spacing between the elements. Frédéric Wang (:fredw) 2014-02-24 02:35:33 PST Comment on [review] MathML mtable changes Review of [review]: ----------------------------------------------------------------- Thanks that looks good to me. Some minor comments below. ::: layout/mathml/nsMathMLmtableFrame.cpp @@ +365,5 @@ > +// Unitless values are permitted and provide a multiple of the default value > +// Negative values are forbidden. > +// > + > +// rowspacing framespacing @@ +386,5 @@ > + > +#define DEFAULT_ROWSPACING_EX 1.0 > +#define DEFAULT_COLUMNSPACING_EM 0.8 > +#define DEFAULT_FRAMESPACING_ARG0_EM 0.4 > +#define DEFAULT_FRAMESPACING_ARG1_EX 0.5 what about using static const variables instead of macros? @@ +435,5 @@ > + } > + > + // Grab the value found and process it. > + if (count > 0) { > + blank line not needed. @@ +474,5 @@ > + > + NS_ASSERTION(aAttribute == nsGkAtoms::rowspacing_ || > + aAttribute == nsGkAtoms::columnspacing_ || > + aAttribute == nsGkAtoms::framespacing_, > + "Non spacing attribute passed"); I would move this assertion at the top of the function body. @@ +494,5 @@ > + aFrame->SetRowSpacingArray(valueList); > + } else if (aAttribute == nsGkAtoms::columnspacing_) { > + aFrame->SetColSpacingArray(valueList); > + } else { // framespacing_ > + if (valueList.Length() >= 2) { I think this should be ==. The spec/RelaxNG schema does not seem to accept more than 2 values. @@ +660,5 @@ > + nsPresContext* presContext = tableFrame->PresContext(); > + if (aAttribute == nsGkAtoms::rowspacing_ || > + aAttribute == nsGkAtoms::columnspacing_ || > + aAttribute == nsGkAtoms::framespacing_ ) { > + nsMathMLmtableFrame* mathMLmtabFrame = do_QueryFrame(tableFrame); could be the full word: mathMLmtableFrame @@ +675,5 @@ > + // clear any cached property list for this table > + presContext->PropertyTable()-> > + Delete(tableFrame, AttributeToProperty(aAttribute)); > + // Reparse the new attribute on the table. > + ParseFrameAttribute(tableFrame, aAttribute, true); So I think these could be reorganized: if aAttribute is nsGkAtoms::*spacing_ => ParseSpacingAttribute else if aAttribute is nsGkAtoms::*align or nsGkAtoms::*lines => ParseFrameAttribute else => ignore attributes that do not affect layout (return NS_OK) @@ +848,5 @@ > { > nsresult rv = nsTableFrame::SetInitialChildList(aListID, aChildList); > if (NS_FAILED(rv)) return rv; > MapAllAttributesIntoCSS(this); > + ParseSpacingAttributes(this); I think the call to ParseSpacingAttributes can be move to MapAllAttributesIntoCSS, so that you don't need to do it here. @@ +857,5 @@ > nsMathMLmtableFrame::RestyleTable() > { > // re-sync MathML specific style data that may have changed > MapAllAttributesIntoCSS(this); > + ParseSpacingAttributes(this); ditto @@ +874,5 @@ > + return 0; > + } > + if (aColIndex < 0 || aColIndex >= GetColCount()) { > + NS_ASSERTION(aColIndex == -1 || aColIndex == GetColCount(), > + "Desired column beyond bounds of table + frame"); do you mean "table frame"? (without the +) ::: layout/mathml/nsMathMLmtableFrame.h @@ +114,5 @@ > + /** helper to get the cell spacing X style value */ > + nscoord GetCellSpacingX(int32_t aColIndex) MOZ_OVERRIDE; > + > + /** helper to get the combined cell spacing X style values of multiple > + * columns perhaps you could say the sum from aStartColIndex to aEndColIndex Frédéric Wang (:fredw) 2014-02-24 03:01:56 PST (In reply to James Kitchener (:jkitch) from comment #19) > The old padding CSS has been stripped out, but I'm not sure if explicitly > zeroing it is correct. > > A consequence of this patch is that css border-spacing on longer has any > effect. IIRC for the other attributes, we tried to preserve the CSS when attributes are not specified explicitly. Maybe we can do the same here. James Kitchener (:jkitch) 2014-02-26 03:08:21 PST Created [review] Part 2: MathML mtable changes The new table spacing code is now opt in. If at least one of rowspacing, columnspacing or framespacing is specified the new spacing is adopted, otherwise it falls back to the old CSS implementation. The code now reports a parsing error if the attribute value contains no lengths (although rowspacing="" will still opt in to the new table spacing system). James Kitchener (:jkitch) 2014-02-26 03:14:03 PST Created [review] Part 3: tests A consequence of the changes in the previous patch is that dir-6 once again fails as it uses the old CSS implementation. The test has been cloned to dir-6a which tests against the new system. Dynamic tests involving transitioning from the old system to new (and vice versa) have been added. James Kitchener (:jkitch) 2014-02-26 17:23:21 PST Comment on [review] Part 2: MathML mtable changes Flagging for review to get confirmation on behaviour change. Frédéric Wang (:fredw) 2014-02-27 05:00:34 PST (In reply to James Kitchener (:jkitch) from comment #26) > Comment on [review] > Part 2: MathML mtable changes > > Flagging for review to get confirmation on behaviour change. OK, change sounds good. I'm expecting that specifiying no attributes behaves the same as the default MathML values. Do you know why it is not working in RTL mode and why your previous patch fixed that? James Kitchener (:jkitch) 2014-02-27 05:27:21 PST Comment on [review] Part1: Add arguments to GetCellSpacing[XY] Cancelling review until further bugs are resolved. James Kitchener (:jkitch) 2014-02-27 05:31:17 PST Comment on [review] Part 2: MathML mtable changes I haven't looked into the rtl problem yet. Cancelling review as the patch is still buggy. setting one of rowspacing/columnspacing/framespacing breaks rowlines and columnlines. rowlines/columnlines draw the lines at the edge of the element, which is no longer the midpoint between two cells. James Kitchener (:jkitch) 2014-03-03 01:24:23 PST Created [review] Part 2: MathML mtable changes This patch applies on top of the patches from bug 975935. The rowlines/columnlines attributes now render properly when used in conjunction with rowspacing/columnspacing/framespacing. The additional area occupied by the row and column lines are considered as overflow for the table cells. James Kitchener (:jkitch) 2014-03-03 01:38:48 PST Created [review] Part 3: tests Now includes a test for combined rowlines/columnlines and rowspacing et al. It blacks out most of an mtable, leaving only the middle exposed. Success occurs if something is drawn in the middle region. I'm avoiding an explicit comparison between a CSS rendered table and a rowspacing etc. one as the two cases may differ due to rounding. James Kitchener (:jkitch) 2014-03-03 01:40:52 PST Comment on [review] Part1: Add arguments to GetCellSpacing[XY] Reflagging for review. James Kitchener (:jkitch) 2014-03-03 01:44:09 PST Created [review] Part 2: MathML mtable changes whitespace fixes. James Kitchener (:jkitch) 2014-03-03 01:48:27 PST Created [review] Part1: Add arguments to GetCellSpacing[XY] whitespace fixes for the lines touched by my code. Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2014-03-03 15:03:59 PST (In reply to James Kitchener (:jkitch) from comment #17) > My understanding is that two accomplish different things. In the attached > diagram: > > left example sets mtd {padding:5px;} > middle example sets mtable {border-spacing:5px} mtd{padding:0px;} > right example sets rowspacing="5px 20px" columnspacing="5px 20px" > framespacing="5px 5px" (using the patches from this bug) > > padding adds the space inside the table cells, which are then placed > touching each other, resulting in a solid block of red. > border-spacing leaves 5px of white between the cells, resulting in a > windowpane effect. > rowspacing/columnspacing is similar to the border-spacing example, except > different spaces can be specified. > > Now I am not an expert in CSS, so the third example might still be > replicable by other means but it may not be a simple process. I think it could be done using invisible, collapsing borders. Could MathJax use that? (In reply to Frédéric Wang (:fredw) from comment #18) > One thing to consider is that in practice we are bit constrained by the > current status of math: > > 1) LaTeX is the de facto standard. And in LaTeX we put all the table > formatting as an argument of the environment command that generates the > table. So it's a bit tedious for LaTeX-to-MathML converters to have to move > all this formating to cell nodes, especially when they are fast stream > filter like itex2MML where you are essentially handling a MathML string not > a DOM tree. That's why LaTeX-to-MathML converters use > columnspacing/rowspacing and the other alignment attributes already > implemented. If I'm right that we could use borders, then you could emit a scoped style sheet and give the cells classes corresponding to row and column numbers to get the streaming behavior. > 2) MathML is still used in environment where CSS is not available. Or in > polyfills like MathJax that don't work well with CSS. In particular, > Wikipedia will rely on MathJax to do server-side LaTeX-to-MathML conversion > and thus the MathML output served will contain columnspacing/rowspacing. I don't like making browser design decisions driven by the needs of platforms where CSS isn't available. This happened in SVG for a long time and it forced duplication of functionality that ended up being pointless since all non-CSS-capable SVG viewers died out. I'm not totally opposed to taking these patches but we have to consider the costs and benefits --- and alternatives --- carefully. dbaron might have an opinion. Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2014-03-03 15:05:11 PST (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #35) > I don't like making browser design decisions driven by the needs of > platforms where CSS isn't available. This happened in SVG for a long time > and it forced duplication of functionality that ended up being pointless > since all non-CSS-capable SVG viewers died out. Actually this isn't completely true, but it's true enough that we stopped caring about non-CSS-capable SVG viewers. Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2014-03-03 15:06:28 PST (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #35) > I don't like making browser design decisions driven by the needs of > platforms where CSS isn't available. This happened in SVG for a long time > and it forced duplication of functionality that ended up being pointless > since all non-CSS-capable SVG viewers died out. Note that one option is that MathJax produces output containing both the attributes and the CSS, and we make sure that UAs support one or the other but not both. James Kitchener (:jkitch) 2014-03-08 01:48:54 PST (In reply to Frédéric Wang (:fredw) from comment #27) > OK, change sounds good. I'm expecting that specifiying no attributes behaves > the same as the default MathML values. Do you know why it is not working in > RTL mode and why your previous patch fixed that? The problem lies in CSS. >mtd:first-child { > padding-left: 0em; >} With dir=rtl this selects the rightmost mtd, rather than the leftmost, so the padding is removed from the wrong side. Frédéric Wang (:fredw) 2014-03-08 23:34:03 PST (In reply to James Kitchener (:jkitch) from comment #38) > The problem lies in CSS. > >mtd:first-child { > > padding-left: 0em; > >} > > With dir=rtl this selects the rightmost mtd, rather than the leftmost, so > the padding is removed from the wrong side. OK, thanks that makes sense. Can you please try to use -moz-padding-start/-moz-padding-end/-moz-margin-start/-moz-margin-end instead? Frédéric Wang (:fredw) 2014-03-08 23:38:19 PST (In reply to Frédéric Wang (:fredw) from comment #39) > (In reply to James Kitchener (:jkitch) from comment #38) > > The problem lies in CSS. > > >mtd:first-child { > > > padding-left: 0em; > > >} > > > > With dir=rtl this selects the rightmost mtd, rather than the leftmost, so > > the padding is removed from the wrong side. > > OK, thanks that makes sense. Can you please try to use > -moz-padding-start/-moz-padding-end/-moz-margin-start/-moz-margin-end > instead? Sorry, I just realized that the problem is with the :first-child selector... (this seems to be one more complication for MathML generators that would like to use CSS instead of rowspacing etc) James Kitchener (:jkitch) 2014-03-10 04:46:42 PDT (In reply to Frédéric Wang (:fredw) from comment #40) > (In reply to Frédéric Wang (:fredw) from comment #39) > > (In reply to James Kitchener (:jkitch) from comment #38) > > > The problem lies in CSS. > > > >mtd:first-child { > > > > padding-left: 0em; > > > >} > > > > > > With dir=rtl this selects the rightmost mtd, rather than the leftmost, so > > > the padding is removed from the wrong side. > > > > OK, thanks that makes sense. Can you please try to use > > -moz-padding-start/-moz-padding-end/-moz-margin-start/-moz-margin-end > > instead? > > Sorry, I just realized that the problem is with the :first-child selector... > > (this seems to be one more complication for MathML generators that would > like to use CSS instead of rowspacing etc) Actually it works. It is a neater solution than relying on :-moz-dir(rtl), which doesn't actually work for MathML due to the way the dir attribute is implemented. James Kitchener (:jkitch) 2014-03-10 06:03:47 PDT (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #35) > I think it could be done using invisible, collapsing borders. Could MathJax > use that? > One potential issue is the rowlines and columnlines attributes. These draw lines between the cells and are currently implemented by us and Webkit by selectively applying table borders. To handle both rowspacing and rowlines with the same border mechanism would require specifying a border that is mostly invisible but has a solid or dashed line in a different colour along one edge. Frédéric Wang (:fredw) 2014-03-10 07:01:47 PDT Note: I agree with Robert that in general we should try to reuse CSS when possible and deprecate MathML features that do not work well with the CSS layout (that was one of the motivation to limit the mstyle support btw). However, in this particular case: a) I have not really studied whether it will be easy/possible to implement that with CSS properties only. b) I'm not sure it will be convenient to handle for MathML generators. I don't understand the "scoped style sheet" since the generators can e.g. just return a $...$ string not a whole HTML document. Also, the style will be lost by copy and paste of the equation, if it is not done by inline style="" attributes. Even if we do that with such attributes, that would make things difficult for stream converters. c) in the short term I think we want to align on what MathJax and current tools do. If a) is not a problem, then in the long term we could either ask to change the spec or try to emulate the specific MathML table layout internally with some Web Components that would automatically do the CSS mapping. Currently, the MathJax native MathML mode has some workaround for rowspacing/columnspacing that adds some CSS style but I'm not sure it always works well (in particular with the rowline/columnline attributes mentioned by James). This CSS style is added neither by MathJax in the SVG/HTML modes nor by the LaTeX-to-MathML server-side conversion that will be used in Wikipedia. If MathJax is changed to add these style in these modes too, I suspect it will conflict with MathJax's own table layout (which does not rely on the built-in CSS table layout). I'm not even sure the MathJax team will agree to modify the preprocessing to always/conditionally add these CSS rules. Finally, we want these attributes to work in other situations and tools where MathJax is not involved. So at the moment, James' patch seems the best (or least bad) option to me. We already have similar things for rowalign/columnalign, except that here it modifies more things outside the MathML code. Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2014-03-24 23:08:27 PDT (In reply to Frédéric Wang (:fredw) from comment #43) > a) I have not really studied whether it will be easy/possible to implement > that with CSS properties only. I think we should study that! If it's hard, I don't mind taking these patches. But if it's easy, I don't think we should take these patches. > b) I'm not sure it will be convenient to handle for MathML generators. I > don't understand the "scoped style sheet" since the generators can e.g. just > return a $...$ string not a whole HTML document. Also, the style > will be lost by copy and paste of the equation, if it is not done by inline > style="" attributes. Even if we do that with such attributes, that would > make things difficult for stream converters. You can write > c) in the short term I think we want to align on what MathJax and current > tools do. If a) is not a problem, then in the long term we could either ask > to change the spec or try to emulate the specific MathML table layout > internally with some Web Components that would automatically do the CSS > mapping. We really don't want to add any Web-exposed features as a short-term solution to anything. They will live forever. > Currently, the MathJax native MathML mode has some workaround for > rowspacing/columnspacing that adds some CSS style but I'm not sure it always > works well (in particular with the rowline/columnline attributes mentioned > by James). This CSS style is added neither by MathJax in the SVG/HTML modes > nor by the LaTeX-to-MathML server-side conversion that will be used in > Wikipedia. If MathJax is changed to add these style in these modes too, I > suspect it will conflict with MathJax's own table layout (which does not > rely on the built-in CSS table layout). I'm not even sure the MathJax team > will agree to modify the preprocessing to always/conditionally add these CSS > rules. Finally, we want these attributes to work in other situations and > tools where MathJax is not involved. I don't understand this. It seems much preferable to change MathJax and other "situations and tools" for MathML than to add unnecessary features to the Web platform. distler 2014-03-25 01:01:37 PDT >I don't understand this. It seems much preferable to change MathJax and other "situations and tools" for MathML than to add unnecessary features to the Web platform. If I may say so, I find this a little confusing. The MathML Specification specifies certain attributes for the elements which are not currently implemented in Gecko. The same effect (you say) can be achieved by judicious use of the HTML5 feature,

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