[dev] CSS Parsing class

[Jan Schneider]

Zitat von Michael M Slusarz <slusarz at horde.org>:

> 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.

And you would all get this as soon as you install Horde_Css_Parser.

>> 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.

Not a version check, but a simple "if  
(class_exists('Horde_Css_Parser'))" switch.

> 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.

Create a topic branch.

>> 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.

It's still a complete replacement of a whole library in a minor bugfix  
version. This is unacceptable. 5.1 is not far away, there is  
absolutely no need for any pressure to get this out now.
-- 
Jan Schneider
The Horde Project
http://www.horde.org/