Bug 105108

Summary: [patch] DOM retrieval of text-shadow property omits commas
Product: [Applications] konqueror Reporter: kirun <bugs_kde>
Component: khtmlAssignee: Konqueror Developers <konq-bugs>
Status: RESOLVED FIXED    
Severity: normal CC: faure, finex, germain, magic, maksim
Priority: NOR    
Version: 3.4   
Target Milestone: ---   
Platform: openSUSE   
OS: Linux   
Latest Commit: Version Fixed In:
Sentry Crash Report:
Attachments: testcase
new testcase
patch which adds a comma, but which breaks style.textDecoration
should fix without breaking other properties
another patch using an enum instead of a DOMString

Description kirun 2005-05-04 21:38:33 UTC 
Version:            (using KDE KDE 3.4.0)
Installed from:    SuSE RPMs
OS:                Linux

When retrieving the text-shadow property value via DOM, any commas are omitted...

e.g.

this.style.textShadow turns style="text-shadow: red 2px 2px 2px, orange 4px 4px 2px" 

into 

red 2px 2px 2pxorange 4px 4px 2px

and not the expected 

red 2px 2px 2px, orange 4px 4px 2px
Comment 1 kirun 2005-05-04 21:39:35 UTC 
Created attachment 10901 [details]
testcase

Click on the text to see the bug
Comment 2 David Faure 2005-05-04 22:19:06 UTC 
Tricky. I thought it was easy to fix in CSSValueListImpl::cssText(), but not all valuelists are comma-separated, apparently.

See new testcase, which shows that text-shadow may be comma separated, but text-decoration is space separated, and both things get stuffed into the same class, so I don't know how to differenciate them. I hope another developer will find out :)
Comment 3 David Faure 2005-05-04 22:20:56 UTC 
Created attachment 10902 [details]
new testcase
Comment 4 David Faure 2005-05-04 22:22:19 UTC 
Created attachment 10903 [details]
patch which adds a comma, but which breaks style.textDecoration
Comment 5 Vincent Ricard 2007-05-06 18:10:21 UTC 
Created attachment 20501 [details]
should fix without breaking other properties

tested with kde4, not on the 3.5 branch
Comment 6 Vincent Ricard 2007-05-12 22:36:40 UTC 
Created attachment 20555 [details]
another patch using an enum instead of a DOMString

This patch replaces my previous one; it's using a enum and a bitfielded member
as discussed with SadEagle on irc.
Comment 7 FiNeX 2008-04-06 18:31:59 UTC 
Has the patch been applied on KDE4 ?
Comment 8 Michael Leupold 2008-04-27 10:35:31 UTC 
Doesn't work for me, neither on 3.5.9 nor on trunk.
Comment 9 Germain Garand 2009-11-08 07:00:37 UTC 
not sure why this patch got forgotten... sorry Vincent ;-/
I'll modify it a bit to add spaces between properties, extend it to other relevant CSS lists I can think of, and commit the result.
Comment 10 Germain Garand 2009-11-08 20:29:01 UTC 
SVN commit 1046456 by ggarand:

need to distinguish at least two types of CSS value lists : those that
use the comma as a separator when flattened to a string, and
those that use the space.

initial patch by Vincent Ricard <magic@magicninja.org>, with input from
dfaure and SadEagle, extended to apply to (hopefully) all relevant
properties.

BUG: 105108

 M  +1 -1      css/css_renderstyledeclarationimpl.cpp  
 M  +1 -1      css/css_svgcssparser.cpp  
 M  +7 -1      css/css_valueimpl.cpp  
 M  +8 -1      css/css_valueimpl.h  
 M  +3 -3      css/css_webfont.cpp  
 M  +5 -5      css/cssparser.cpp  
 M  +1 -1      svg/SVGFontFaceElement.cpp  
 M  +1 -1      svg/SVGFontFaceSrcElement.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1046456