[dev] CSS Parsing class

Michael M Slusarz slusarz at horde.org
Tue Mar 19 21:03:30 UTC 2013


Quoting Jan Schneider <jan at horde.org>:

> I don't feel comfortable with replacing the CSS parser in a bug fix  
> release of Horde_Core.

Disagree.

This new parser is NOT adding any new features.  It is a bug fix.  It  
fixes several (critical) bugs:

1. Smartmobile view is currently NOT cacheable because we have to  
explicitly disable parsing.  This is a pretty huge deal.  Now that we  
allow smartmobile to be themed, there is no guarantee that these  
changes will appear on a user's browser.  Which is something we  
GUARANTEE when using CSS caching.  This is *totally* broken behavior.   
This is unacceptable.
2. People are starting to develop new themes.  Nowhere in  
documentation/API have we said that CSS3 features (or at least large  
portions of it) can't be used.  But that's currently the case.  That  
is unacceptable.  This is not something that should wait for 5.1.
3. While making this transition, I discovered an additional limitation  
with csstidy regarding parsing urls that can't be fixed without  
substantial work.
4. In IMP, I discovered that CSS styles could potentially leak into  
HTML output because csstidy can not properly parse it.

> Can we instead keep a conditional in the client code until the 5.1  
> release which checks whether Horde_Css_Parser exists and falls back  
> to Csstidy if not?

Csstidy is not used in client code other than IMP.  And as mentioned  
above, using the new code in IMP fixes at least one security-related  
issue (better described as a privacy issue, but still).

And in Horde_Core... we most certainly don't want to be doing some  
sort of horde application version check >= 5.1.  That sort of defeats  
the whole purpose of separating applications and PEAR packages.

It also demonstrates a limitation with our current git layout.  For  
example... I really want to update jquerymobile to 1.3.0 to allow for  
implementation of the IMP smartmobile tablet view.  But there's not  
really a coherent way to accomplish this.

> We can make Css_Parser an optional dependency then and release beta  
> versions first for people to test this more thoroughly. With 5.1 we  
> completely switch to the new parser.

I've already tested and verified via diff tool comparison that the new  
code is already outputting the same output for translating URLs and is  
correctly recognizing/importing external stylesheets.  So its already  
doing everything the current code does.  Anything else will just be a  
bonus.

michael

___________________________________
Michael Slusarz [slusarz at horde.org]



More information about the dev mailing list