The attached testcase sets the rowlines and columnlines attributes for an mtable. In Beta (28.0) the lines appear as expected. In Aurora (29.0a2) and Nightly (30.0a1) no lines appear (other than the exterior border) and the table is indistinguishable from one without the rowlines/columnlines attributes.

Possible regression window. Dec-13 Nightly: Mostly working (although the lines may disappear when reloading). Dec-14 Nightly: Not working. Bug 731667 perhaps?

It appears that nsDisplaymtdBorder::Paint() is never actually called. The class however is created when nsMathMLmtdFrame::ProcessBorders() is called. It is possible that it is somehow omitted from the list of items sent to FrameLayerBuilder::PaintItems(), as I have been unable to detect any item with class nsDisplaymtdBorder when run under a debugger. (It isn't hiding under a nsDisplayBorder classname as I commented out the instances where the parent class is created prior to testing).

The problem is that when the code checks for the visibility of the border, it checks a data structure that hasn't been updated to include the <mtd> border. As it isn't visible the border drawing is skipped. This code overrides GetBounds to take into account the newly added <mtd> border. (As well as some minor refactoring to nsDisplayBorder to reduce code duplication).

Comment on attachment 8383470 [details] [diff] [review] Part 1: nsDisplay(mtd)Border changes Review of attachment 8383470 [details] [diff] [review]: ----------------------------------------------------------------- BTW it looks to me like nsMathMLmtdFrame needs a Reflow method that sets its overflow areas to include this extra border. ::: layout/base/nsDisplayList.cpp @@ +2671,5 @@ > nsDisplayBorder::GetBounds(nsDisplayListBuilder* aBuilder, bool* aSnap) > { > *aSnap = true; > + const nsStyleBorder styleBorder = *mFrame->StyleBorder(); > + return CalculateBounds(styleBorder); You're making an unnecessary copy here. Just pass *mFrame->StyleBorder() directly

Tests to ensure the regression doesn't happen again. These tests have been imported from MathJax, with minor modifications to adapt it to our setup. The tests ensure that the attributes actually do something, that solid lines are solid and that dashed lines are dashed. Apache 2.0 licensed.

Comment on attachment 8383472 [details] [diff] [review] Part 2: Tests Review of attachment 8383472 [details] [diff] [review]: ----------------------------------------------------------------- I think the comment should be "more than 1px" rather than "at least 1px". I'm a bit concerned that the reftest will fail because of antialiasing and stuff, but let's see the result on the try server. Probably for completeness we should have columnlines-3* too.

Now includes a change to make reflow aware of the additional border.

The additional tests have been added. Green try run https://tbpl.mozilla.org/?tree=Try&rev=1a249583cda1

https://hg.mozilla.org/integration/mozilla-inbound/rev/a3985ae9699a https://hg.mozilla.org/integration/mozilla-inbound/rev/a3ac147bff3f

https://hg.mozilla.org/mozilla-central/rev/a3985ae9699a https://hg.mozilla.org/mozilla-central/rev/a3ac147bff3f

[Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 731667 regression User impact if declined: rowlines and columnlines attributes on MathML mtables will stop working Testing completed (on m-c, etc.): Landed on m-c a few days ago without issue, verified the latest nightly works appropriately. Patch also includes comprehensive tests. Risk to taking this patch (and alternatives if risky): Low - impact limited to the broken attributes. String or IDL/UUID changes made by this patch: None

(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #9) > https://hg.mozilla.org/integration/mozilla-inbound/rev/a3985ae9699a I've just realised this landed with the wrong reviewer information (r=bz when it should have been r=roc). My apologies.