Bug 105108 - [patch] DOM retrieval of text-shadow property omits commas
Summary: [patch] DOM retrieval of text-shadow property omits commas
Status: RESOLVED FIXED
Alias: None
Product: konqueror
Classification: Applications
Component: khtml (show other bugs)
Version: 3.4
Platform: openSUSE Linux
: NOR normal
Target Milestone: ---
Assignee: Konqueror Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-05-04 21:38 UTC by kirun
Modified: 2009-11-08 20:29 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
testcase (369 bytes, text/html)
2005-05-04 21:39 UTC, kirun
Details
new testcase (547 bytes, text/html)
2005-05-04 22:20 UTC, David Faure
Details
patch which adds a comma, but which breaks style.textDecoration (499 bytes, patch)
2005-05-04 22:22 UTC, David Faure
Details
should fix without breaking other properties (1.50 KB, patch)
2007-05-06 18:10 UTC, Vincent Ricard
Details
another patch using an enum instead of a DOMString (1.80 KB, patch)
2007-05-12 22:36 UTC, Vincent Ricard
Details

Note You need to log in before you can comment on or make changes to this bug.
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