Testcase coming up. Using a reframe hint for mRules changes would probably work, but that really should not be necessary, should it?

setAttribute(rules' is that missing ' intentional?

No, it's not... it's a typo. The rest of the testcase is not affected by that, though.

Believe that the testcases at http://lxr.mozilla.org/seamonkey/source/layout/html/tests/table/collapsing_borders/bug41262-2.html have worked once, this looks like a regression.

Indeed... Looks like changing the "rules" attribute in fact triggered a framechange hint before the checkin for bug 211308. See http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsHTMLTableElement.cpp&branch=&root=/cvsroot&subdir=mozilla/content/html/content/src&command=DIFF_FRAMESET&rev1=1.123&rev2=1.124

setting the rules attribute invokes border collapse.

we need to frame change see: http://lxr.mozilla.org/seamonkey/source/layout/html/table/src/nsTableCellFrame.cpp#1211

"Invokes border-collapse" is not sufficient here -- there is a rules attr on the table when the page is loaded (albeit an empty one), so the table starts out in the collapsing borders model. So why is a reframe required at all? We're already doing border-collapse: collapse. So changing the rules should be a matter of repainting the borders, no? I checked, and the post-resolve callbacks in nsHTMLStyleSheet are being called and the border data is being reset in ProcessTableRulesAttribute for the new style context.... do tables somehow cache that border information somewhere and not let go? I couldn't find any accesses to mRules anywhere outside the post-resolve callback code... I suppose we can check this in if all else fails, but it'd be nice to know what's going on. ;)

Ah, ok. I found it. See nsTableFrame::Reflow -- it checks NeedToCalcBCBorders() before recalculating them... and the BC border cache is not invalidated by a change in mRules (since a simple style change reflow does not invalidate it).

Comment on attachment 136213 [details] [diff] [review] This does fix the bug... I think this is the right patch. The alternative is to blow away the BC border cache on every style-change reflow, and that's prohibitively expensive, I would guess... whereas dynamic changes in the "rules" attribute happen once in a blue moon.

Taking. I think we should take this for 1.6... not sure about the state of 1.6b, though.

Boris could you also move the comment from http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/html/content/src/nsHTMLTableElement.cpp&mark=1490#1485 in to your patch? The align caused also a FrameChange hint, is that still the case or is that another opportunity for a possible regression?

I actually did add a comment to that effect in my patch... I should probably remove it from nsHTMLTableElement. Note that the comment in nsHTMLTableElement is NOT correct, since here we have a case that requires a reframe even though border-collapse is already in effect. Align is just mapped into floating, which causes a reframe when turned on/off as it is. So that part is fine.

Comment on attachment 136213 [details] [diff] [review] This does fix the bug... just remove the comment from the table element, btw the comment is "exactly" saying that unfortunetaly this is necessary even if the table is already bc, (You might need some special education to be able to get the deeper meaning of Chris comments, but I got that over the years)

Comment on attachment 136213 [details] [diff] [review] This does fix the bug... Could this please be approved for 1.6? This fixes a regression in dynamic changes to the "rules" attribute. This is a very safe patch -- it just makes us reconstruct the table layout object when "rules" changes.

Comment on attachment 136213 [details] [diff] [review] This does fix the bug... a=brendan@mozilla.org for 1.6b. /be

Checked in for 1.6b

Is this bug (and its fix) having an impact on bug 155507 or maybe bug 172213? Just asking. Thanks.

No.

The table rules are not visible when when clicking the "All borders" button of attachment 136185 [details]. Seamonkey 1.1a rv: 1.9a1 build 2005092405 under XP Pro SP2 here.

That's a regression from the fix for bug 155507. Could you please file a bug and cc me, dbaron, and bernd on it?

Done. I filed bug 310100.

I have filed bug 310127. Is it ok if I cc you 3 on it? Just asking...