Bug 105108

Summary:	[patch] DOM retrieval of text-shadow property omits commas
Product:	[Applications] konqueror	Reporter:	kirun <bugs_kde>
Component:	khtml	Assignee:	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:
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