Feature Proposal: Teach TablePlugin CSS

Motivation

Improve look and feel of TWiki

Description

By default TWiki renders tables using the TablePlugin's default settings. These are completely out of the scope of a skins CSS stylesheet.

The html code this plugin generates is not tagged with apropriate CSS clases.

There's a patch available at TablePluginDev that adds these features.

There's another interesting CSS patch for the TablePlugin that allows per-cell styles. I propose to merge this and my patch and then port it to the TablePlugin in the DevelopBranch and sync everything back to twiki.org.

This feature is need for the upcoming NatSkin for DakarRelease.

-- MichaelDaum - 02 Jul 2005

Impact and Available Solutions

Note: Patch is attached as https://twiki.org/p/pub/Codev/CompleteCssControlofTWikiTables/twiki-foo-bar-patch.diff. The patch is against the TWikiAlphaRelease of 15 Feb 2004.

Documentation

If necessary, user documentation of new features introduced by this proposal.

Examples

Example uses of features introduced by proposal.

Implementation

Any comments on how the feature is implemented or could be improved

---

Discussion:

does any of this allow giving tables classes? So admins can create css for specific table classes and then people can create tables of those classes?

I suppose a workaround for now would be to enclose the tables in a div of a specific class and add rules to the css like div.someclass table { ... }.

-- SamHasler - 08 Jul 2005

My main concerns about the current TablePlugin is that it is non-controlable by skins. TWiki tables should follow the design of a skin with no extra provisions what so ever by the user. Just make your pipes and the tables won't drop out of the skin design. Switching the skin or a stylesheet will adjust the look of the tables as well.

To put it more harsh: no html markup emitted by the TWiki engine should ever be un-classified.

-- MichaelDaum - 08 Jul 2005

... and the table markup emmited by the Twiki engine and the plugins sghould not be in conflict with CSS. That is, it should NOT emit

  <table height="..." 

but instead

   <table style="height: ....

-- AntonAylward - 09 Jul 2005

Here's what my patch to the TablePlugin already allows:

%TABLE{tableclass="twikiTable myTableClass" dataclass="myTableCell1, myTableCell2"}%
| *hello* | *world* |
| hello | world |
| hello | world |
| hello | world |
| hello | world |
| hello | world |
| hello | world |

.myTableClass {
  width:100%; /* SMELL: does not work with pattern skin */
  border:1px solid gray;
}
.myTableClass th {
  border-bottom:1px solid gray;
  background:lightgray;
}
.myTableClass td {
  padding:5px;
}
.myTableCell1 {
  color:red;
}
.myTableCell2 {
  background:orange;
}
</style>

So, MOST important it renders a

<table class="twikiTable...

and you get (simulated)

hello	world
hello	world
hello	world
hello	world
hello	world
hello	world
hello	world

You could even put your custom table styles on a MyTableStyles page and %INCLUDE% it on every page where you want to have this style, being able to change all of the table's appearance in a single place.

-- MichaelDaum - 09 Jul 2005

Here's a demo of what I mean. This is a cairo engine with a preview of the upcomming NatSkin and the TablePlugin patch already applied. Use the different skin styles and what how the tables always stays in sync with the skin style.

-- MichaelDaum - 09 Jul 2005

You shouldn't have to include CSS in topics, there should be a more standard syntax to allow adding links to CSS files to the header. I've discussed this before elsewhere and I will pick it up again. I'm just saying so you know.

-- SamHasler - 12 Jul 2005

I agree with Sam. Micha's example is a kluge. Strictly speaking CSS, and javascript for that matter, shouldn't be allowed outside the header.

That browsers allow it, that they honour it, is an interesting anomaly. It is not one we should rely on as an underpinning for design.

-- AntonAylward - 13 Jul 2005

There is a "hack" (found in EditTablePlugin) that allows plugins to put stuff in the header. PeterThoeny commented about this in some other topic, but as I couldn't find it again I paste the code below:

sub commonTagsHandler {

    if(  $_[0] =~ m/^[<][!]DOCTYPE/ ) {

        my $string = " <link type=\"text/css\" rel=\"stylesheet\""
                   . " href=\"$mishooHome/calendar-system.css\" />\n";
        $_[0] =~ s/([<]\/head[>])/$string$1/i;
    }
}

While this is a solution, I agree that there should be a more standard way to add stuff to the header.

-- RafaelAlvarez - 13 Jul 2005

Maybe adding javasript or css to the topic text itself is a kludge. But every browser I've seen so far copes with it the expected way.

Doing this standards-conformantly is not possible with TWiki because each html is rendered in three steps (a) render everything before %TEXT%, (b) render %TEXT% and (c) render things after %TEXT% each of which establishes a context header_text, body_text and footer_text respectively.

There's NO way to add constraints to the header_text from within the body_text context.

What would be an intuitive solution was to collect all

%STARTCSS%
.class1 {...}
.class2 {...}
%STOPCSS%

sections from the body_text and similarly

%STARTJAVASCRIPT%
function blabla() {
}
%STOPJAVASCRIPT%

and then insert them at the end of the <head> ... </head> section with some similar code as the one shown by Rafael. Sorry, no go with TWiki Once a plugin gets the body_text context the stuff produced in the header_text context is frozen solid.

The only way to solve this right now is to establish a loophole so that the client imports external css/javascript files (see UsingTopicToDefineCSS) which are to be transmitted to the client in one or more extra connections. And then generate this extra css/javascript resources on the fly containing the collected data picked up during STARTCSS...STOPCSS STARTJAVASCRIPT...STOPJAVASCRIPT. Note, however that the header_text part must not be transfered to the client beforehand because of the obvious racecondition that might appear.

So we are free to replace once kludge with another given the engine stays as it is.

-- MichaelDaum - 13 Jul 2005

The point SamHasler made is a good one: Users shouldn't be able to add javascript and css to topics. It's too "dangerous".

-- RafaelAlvarez - 13 Jul 2005

Why is it "dangerous" to write css? Sorry, but I don't understand. Is it in terms of ruining the page layout, or in terms of browser security, or in terms of getting even more disarranged users? You could ruin the layout with illegal html inside the topics aswell ... If it was that dangerous then we need to filter out css and javascript automatically.

-- MichaelDaum - 13 Jul 2005

sorry, I mean browser security. AFAIK, you can inject javascript (or use an htc file in IE) to a page using CSS, even if javascript is locked down.

-- RafaelAlvarez - 13 Jul 2005

Wow, so again, any wiki not filtering javascript+css from topic texts will be a threat to IE?

-- MichaelDaum - 13 Jul 2005

basically yes, as is any website not smart enough to allow user enter javascript+css into a comment form. And not only a threat to IE, there are some attacks using javascript that affect any browser (like the document.location=something attack).

-- RafaelAlvarez - 13 Jul 2005

Could you please point me to a recent advisory about javascript vulnerabilities that is not about five years old and has not been resolved in current browsers.

So this is a general question of every Wiki's security armor. There might be a browser threat by allowing any user to insert malicious html tags somewhere (topic text, comment fields, etc) but also by simply using INCLUDE or plugins like the beloved HeadlinesPlugin to integrate html from a third party...

Nevertheless, to come back to the actual proposal made here, it is a bad idea for the TWiki engine to emit html that is not classified. Please, let's put at least a class="twikiTable" into each table rendered by the TablePlugin.

-- MichaelDaum - 13 Jul 2005

Yes, Micha, there are gaping holes in security that need aggressive filtering. This was discussed in the aftermath of the %SEARCH security bug.

See my comments in RequireEXPLICITTagToEnableHTML about > and < and UTF-7 and my subseqinet comments about TWiki following Microsoft's approach of putting features ahead of integrity - something that, thankfully, CrawforCurrie and the other DakarRelease luminaries are resisting.

See also UsersCanPutJavascriptInTopics and the mention of HTML::Scrubber::StripScripts

See: http://www.modsecurity.org/, which I discuss and recommend in a number of places.

See AndyGlew's comments at ScriptsAreResponsibleForOwnSecurity

But yes, I agree. Please, let's put at least a class="twikiTable" into each table rendered by the TablePlugin.

-- AntonAylward - 13 Jul 2005

There's one issue left with the TablePlugin: there are hard-coded defaults showing up if you uncomment the TABLEATTRIBUTES. That's bad looking through the skin author's lenses. So let's get rid of them.

Finally, whether users are allowed or even encouraged to add css to their topics is not in the scope of this proposal any more.

If there are no more objections against the given patch I'd like to check that in.

-- MichaelDaum - 14 Jul 2005

And btw: can we remove html color and border attributes that are hard-coded into the TWiki engine, please. This should be left untouched or classified using some css tags.

Example: the forms. Their form data has a border of 1px by default. What if I don't like it?

This will patch it away.

--- Render.pm   (revision 4639)
+++ Render.pm   (working copy)
@@ -222,7 +222,7 @@
         }
     }
     return CGI::div( { class => 'twikiForm' },
-                     CGI::table( { border => 1 }, $metaText ));
+                     CGI::table( { }, $metaText ));
 }

Similarly, there are more questionable html attrs in TWiki/Form.pm, TWiki/UI/Edit.pm, TWiki/UI/RDiff.pm and TWiki/Render.pm. We should move that into some stylesheet.

-- MichaelDaum - 14 Jul 2005

While I totally agree that the TABLEPLUGIN tables should be enhanced with css classes, be careful not to remove all styling; some people will still use classic skin. Instead write class tags around or inside the html attributes so that they can be overridden easily.

At leasts pages in classic skin should degrade peacefully.

-- ArthurClemens - 14 Jul 2005

Ah, that's the problem, Arthur.

The CGI:: tools don't "do class".

Or as the snobs used to say: "NQOC" - 'Not Quite Our Class'

-- AntonAylward - 14 Jul 2005

How don't they do class? I can use:

CGI::span( {class => 'twikiError' }, "Incorrect password" )

-- ArthurClemens - 14 Jul 2005

Yes. You can.

Doing a grep I get:

./TWiki/UI/Edit.pm:        $formText = CGI::Tr(CGI::td( { align=>'right' }, $formText ));
./TWiki/UI/Edit.pm:        $formText = CGI::table( { width=>'100%',

./TWiki/UI/RDiff.pm:    my $d1 = CGI::td({ bgcolor=>$format{$lc}[0],
./TWiki/UI/RDiff.pm:    my $d2 = CGI::td({ bgcolor=>$format{$rc}[0],


./TWiki/UI/RDiff.pm:    my $d1 = CGI::td({ bgcolor=>$format{$lc}[0],
./TWiki/UI/RDiff.pm:    my $d2 = CGI::td({ bgcolor=>$format{$rc}[0],


./TWiki/UI/RDiff.pm:                           CGI::th({align=>'center'},
./TWiki/UI/RDiff.pm:                           CGI::th({align=>'center'},

./TWiki/UI/RDiff.pm:                           CGI::th({align=>'center'},
./TWiki/UI/RDiff.pm:                           CGI::th({align=>'center'},


./TWiki/UI/RDiff.pm:        $row = CGI::td({bgcolor=>$format{$code}[0],

./TWiki/UI/RDiff.pm:        return CGI::Tr(CGI::td({bgcolor=>$bg,


./TWiki/UI/RDiff.pm:        $result .= CGI::Tr({bgcolor=>$format{l}[0],
./TWiki/UI/RDiff.pm:                           CGI::th({align=>'left',
./TWiki/UI/RDiff.pm:            $result .= CGI::Tr(CGI::td({ width=>'50%'}, ''),
./TWiki/UI/RDiff.pm:                               CGI::td({ width=>'50%'}, ''));
./TWiki/UI/RDiff.pm:            $result .= CGI::Tr(CGI::td({ width=>'50%'}, ''),
./TWiki/UI/RDiff.pm:                               CGI::td({ width=>'50%'}, ''));


./TWiki/Plugins/TestFixturePlugin.pm:          CGI::Tr({}, CGI::td({valign=>'top', colspan=>2},
./TWiki/Plugins/TestFixturePlugin.pm:                              CGI::code($a)));
./TWiki/Plugins/TestFixturePlugin.pm:        $opts->{result} .= CGI::Tr({valign=>'top'},
./TWiki/Plugins/TestFixturePlugin.pm:                                   CGI::td({bgcolor=>'#99ffcc'},CGI::pre($a)));
./TWiki/Plugins/TestFixturePlugin.pm:        $opts->{result} .= CGI::Tr({valign=>'top'},
./TWiki/Plugins/TestFixturePlugin.pm:                                   CGI::td({bgcolor=>'#ffccff'},CGI::pre($b))


./TWiki/Plugins.pm:    my $text = CGI::start_table( { border => 1 } ).
./TWiki/Plugins.pm:      CGI::Tr(CGI::th('Plugin').CGI::th('Errors'));
./TWiki/Plugins.pm:        $text .= CGI::Tr( { valign=>'top' },
./TWiki/Plugins.pm:                          CGI::td($plugin->{name}). CGI::td( $err ));
./TWiki/Plugins.pm:    $text .= CGI::end_table().CGI::start_table({ border=>1 }).
./TWiki/Plugins.pm:      CGI::Tr(CGI::th('Handler').CGI::th('Plugins'));
./TWiki/Plugins.pm:            $h = join( CGI::br(),
./TWiki/Plugins.pm:        $text .= CGI::Tr( { valign=>'top' },


./TWiki/Render.pm:            $metaText .= CGI::Tr( { valign => 'top' },


./TWiki/Render.pm:                     CGI::table( { border => 1 }, $metaText ));
./TWiki/Render.pm:          CGI::start_table({ border=>1, cellspacing=>0, cellpadding=>1} );


./TWiki/Form.pm:    my $text = CGI::start_table(-border=>1, -cellspacing=>0, -cellpadding=>0 );
./TWiki/Form.pm:    $text .= CGI::Tr( CGI::th( { colspan => 2,
./TWiki/Form.pm:            $text .= CGI::Tr(CGI::th( { align => 'right',
./TWiki/Form.pm:                             CGI::td( { align=>'left' } , $value ));

Yes. YOU can. However the current code-base uses the non- style constructs that, as I pointed out above (9th July 2005), are not compatible with style-sheets.

To be fair, I just grepped for non-style instances of CGI::. I'm sure some of them are as they should be; some tables should always be 100%, some rows should always have that vertical alignment.

But there's a world of difference between can and do.

-- AntonAylward - 14 Jul 2005

I'm on holiday in denmark for two weeks. Can someone take take of the attached patch to the TablePlugin? I'd realy like to see that in the final release. IMHO classes for even and odd rows of a table would have been nice too, but I didn't find the time to do that. See y'all in two weeks.

-- MichaelDaum - 16 Jul 2005

Filed Bugs:Item299

-- MichaelDaum - 24 Aug 2005

twikiTable has been added to the core renderer and the TablePlugin. The rest of the proposed patch is withdrawn.

-- MichaelDaum - 28 Aug 2005