r107669 MediaWiki - Code Review archive

Did you test this ? Last I checked browsers insert (if not already) a TBODY element in between there

Added complementary rules in r107673.

Nice :)

Perhaps we can try with only the ..table > tbody > tr .. version, that one should work in firefox too.

Apparently Firefox has some special thingy that allows styles for ..table > tr.. to work also.

(by the way for more info, see here)

Having both is being surely compatible with all browsers supporting > selector thus staying on the safe side.

I tested with only the ..table > tbody > tr .. version in browserstack and works successfully in Chrome, Firefox 3.6+, IE7+ and Opera 10.5+ (maybe earlier version of Opera too but don't have an older version).

This version does not work in IE6, but does the current version in SVN with both also does not work in IE6 (it does not support the > child selector and skips this selector entirely).

According to the W3 specification user agents must and do wrap tables in tbody if no table containers are specified in the document (thead, tbody, tfoot). So we're safe there. However, as you mentioned on IRC, this is only true in HTML mode.

MediaWiki is in HTML5 by default, but wikis can be put it in XHTML1 mode, and in that mode, according to the specification, user agents must not do these kind of changes.

So we've got a situation here in which we will break:

• In IE6, which we still officially support and tend to provide complete functionality
• Existing styles on wikis that use .wikitable tr etc. (can be fixed, but this is done a lot, we need early announcement)
• Existing pages on wikis that assume childtables will automatically get the same styling

And it fixes:

• No longer apply wikitable styles to nested tables that do not have a wikitable class.

(...and it fixes:)

• excessive superfluous useless (ab)using of !important switch for nested things.

"Existing pages on wikis that assume childtables will automatically get the same styling" - that's actually wrong implied presumption based on the wrong original behavior. There should not be any automatical inheritance, all nested tables (apart from it being evil) should have explicitly defined wikitable class if they are intended to look the same. So it is easily fixable on wiki as well.

I agree it is wrong to assume that kind of inheritance, I'm saying that users are currently seeing that and because that's what it does, it is not unreasonable for people to expect that after so many years.

Surely it's not a reason to not break it, simply something to keep in mind that users need to be told in advance so that it can be fixed where needed.

The way I usually go about these kind of changes is to put them in MediaWiki:Common.css of commons.wikimedia and en.wikipedia and announce it in the Village Pump and see if there are complaints. If not, they can stay until after 1.19 is deployed. That ensures a smooth transition.

I'm ready to jump in and check (and eventually correct) all wikis when it's deployed.

Is that revision resolved with the follow up or should we revert it for now?

this revision's issues are solved, but there is still a point unanswered. And that is that this revision makes wikitable styles no longer apply to IE6, which as of writing it still supported for A-grade experience. Reason being the use of child selectors in CSS.

Well, serving HTML5 on one side and supporting IE6 on the other is quite a big paradox.

By the way, this isn't the only IE6 problematic declaration, for instance .catlinks have display: inline-block; and its support in IE6 is problematical.

If there was some default setting of table known, then in IE6 specific stylesheet it would be possible to emulate the child selector by reverse override via successor selectors, so something like:

table.wikitable td {
  border: 1px solid #aaa;
}
table.wikitable td td {
  border: none;
}

However I think it's time to finally drop the 100 % support for IE6 and drop the support completely within this year. But that's for another discussion.

Besides, this is also solving several additional bugs (see those two listed in summary as an example) in most majority browsers, which is a huge benefit which way so much overweights possible negatives coming from different rendering in IE6.

With the CSS definition above you can't emulate a child selector completely. The following nested table lose the cell border in the inner table:

{| class="wikitable"
| outer table
|-
|
{| class="wikitable"
| inner table
|-
| second row
|}
|}

Here are some examples to emulate a CSS child selector in IE6: http://craftycodeblog.com/2010/05/19/emulating-css-child-selectors-in-ie6/

I think the jQuery workaround would be applicable for svn:trunk/phase3/skins/common/IEFixes.js:

$( "table.wikitable > * > tr > td" )
	.css( "border", "1px #aaa solid" )
	.css( "padding", "0.2em" );

Yup that's the site I originally read, but then I lost it from my bookmarks and the only think I remembered was to restyle nested stuff with default settings. So I wrote the above code as an example of what I mention, not necessarily like what it should be. Thanks for the link.

So now the question is - should we use pure CSS, or count on jQuery?

The question is pure CSS, CSS with expression oder JavaScript. jQuery is just JavaScript. expression is a Microsoft specific extension witch also requires activated JavaScript. This requirement is not mentioned in the blog. With pure CSS you need for every depth of nested tables a special rule with descendant selectors. This increases the specificity for the rules and generates problems with other CSS definitions. The jQuery script above overrides the inline CSS definitions:

This is also not mentioned in the blog. With JavaScript it should be possible to care about not overwriting inline CSS. Maybe it is possible to use expression without knowing the fallback style.

I think there is no easy workaround for IE6.

> I think there is no easy workaround for IE6.

By which we are again hitting the question of dropping of 100% IE6 support. Then the IE6fixes could behave in the old way (like with the descendant instead of children selector).

PS: And yes, I know what jQuery and expressions are and what are the requirements ;-)

Using expressions looks hopeful.

/* for all but IE */
#nav ul li.currentpage > a:hover {
  background-color: #eff;
}

/* for IE */
* html #nav ul li.currentpage a:hover {
  background-color: expression(/currentpage/.test(this.parentNode.className)? "#eff" : "#ef0");
}

Source: http://stackoverflow.com/a/1593260/319266

We'd use it to get the classname of this.parentNode.parentNode.parentNode.className. However the problem is indeed getting a sensible fallback. Not to mention the existing styles on people's wikis, migrating those would become cumbersome.

Perhaps we can simply keep the old behavior (which we've used to date for many many years) as fallback in IE6 ?

> Perhaps we can simply keep the old behavior (which we've used to date for many many years) as fallback in IE6 ?

table.wikitable td { border: 1px #aaa solid; padding: 0.2em; }

just for IE6? Then we have still can't remove the !important rules.

The fallback value is not known. Is it possible to use expression without a fallback value?

But we could move those !important rules to IE6 specific stylesheets to prevent cluttering of the regular one as well as to have it simply cuttable-off once we'll drop the support for IE6 completely (which hopefully will be soon).

> The fallback value is not known. Is it possible to use expression without a fallback value?

Hm.. it's not possible to conditionally add a declaration as the expression can only be used in or as part of the value. However, my experience has always shown that browsers nicely ignore and skip rules it doesn't understand. I wonder to what extend something like the following would match the absence of the rule:

table.wikitable > * > tr > td {
  border: 1px #aaa solid;
  padding: 0.2em;
}

/* IE6 only */
* html table.wikitable * tr td {
  border: expression(/wikitable/.test(this.parentNode.parentNode.parentNode.className)? "1px #aaa solid" : "ignorethisinvalidvalue");
  padding: expression(/wikitable/.test(this.parentNode.parentNode.parentNode.className)? "0.2em" : "ignorethisinvalidvalue");
}

When this works this looks like a adequate workaround for IE6.

Needs to check if IE6 adds or not the tbody, thead and tfoot. Or simply have doubled rules, as they are now.

expression overwrites the inline CSS definition.

I used this configuration with /(^| )wikitable2($| )/ as regular expression. Here is my test case:

Maybe this is avoids overwriting an inline CSS definition if given (untested):

/* IE6 only */
* html table.wikitable * tr td {
	border: expression( ! this.style.border && /(^| )wikitable($| )/.test( this.parentNode.parentNode.parentNode.className ) ? "1px #aaa solid" : "ignorethisinvalidvalue" );
	padding: expression( ! this.style.padding && /(^| )wikitable($| )/.test( this.parentNode.parentNode.parentNode.className ) ? "0.2em" : "ignorethisinvalidvalue" );
}

Danny B. wrote:

table.wikitable td {
  border: 1px solid #aaa;
}
table.wikitable td td {
  border: none;
}

I would do it the other way around, apply a default style to nested childs then apply the style we want.

/** First apply default style to any nested childs */
table.wikitable tr * td {
 border: none;
}
/** then the specific one */
table.wikitable tr td {
  border: 1px solid #aaa;
}

I just found this *after* I filed bug 33752 to revert this change. Now I know the reasoning, perhaps there is another solution. As long as no one has the cojones to take IE6 off the list of supported browsers, this is a major breaking change.

However, instead of using '>' selectors, there is another way. The following mill reset any styling of nested wikitables. In effect, it will have the same result as the code currently in trunk, without breaking IE6:

table.wikitable th,
table.wikitable td {
	border: 1px #aaa solid;
	padding: 0.2em;
}
table.wikitable table.wikitable th,
table.wikitable table.wikitable td {
	border: none;
	padding: 1px;
}
table.wikitable th {
	background-color: #f2f2f2;
	text-align: center;
}
table.wikitable table.wikitable th {
	background-color: transparent;
}
table.wikitable caption {
	font-weight: bold;
}
table.wikitable table.wikitable caption {
	font-weight: normal;
}

I'd much prefer loading CSS specific for IE6. All suggestions are difficult to read and will have so many specificity or other side effects, which will be even harder to debug if encountered by dumb luck. It's making the code more complicated than should be required.

Forget my code above. Does not have the same effect.

For large tables td { expression() } can be a performance problem.

An other idea is to use the HTML attributes border and cellpadding for IE6:

$( 'table.wikitable' )
	.attr( 'border', '1' )
	.attr( 'cellpadding', '2' )

in stead of

table.wikitable > * > tr > th,
table.wikitable > * > tr > td {
	border: 1px #aaa solid;
	padding: 0.2em;

Test case:

paste to address bar: (untested in IE6)

javascript:void( $( 'table.wikitable3' ).attr( 'border', '1' ).attr( 'cellpadding', '2' ) )

We discussed this in the CodeReview meeting and decided to mark this OK, without the expectation to be adding work-arounds for IE6 that complicate the cascading flow of the css for wikitables. IE6 would fallback to default table styling.

> We discussed this in the CodeReview meeting and decided to mark this OK

This is probably the smartest move. I guess we should drop IE 6 support soon.