Bug 14645 - getPropertyValue should be case insensitive
Summary: getPropertyValue should be case insensitive
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Nobody
URL: http://testserver.se/webkit/tests/get...
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2007-07-17 12:10 PDT by Olle Lundberg
Modified: 2007-08-16 04:56 PDT (History)
3 users (show)

See Also:


Attachments
Test case (356 bytes, text/html)
2007-07-17 12:11 PDT, Olle Lundberg
no flags Details
Proposed patch (32.81 KB, patch)
2007-08-06 06:26 PDT, Andrew Wellington
sam: review-
Details | Formatted Diff | Diff
Propose patch 2 (51.99 KB, patch)
2007-08-11 04:37 PDT, Andrew Wellington
mrowe: review+
Details | Formatted Diff | Diff
Proposed patch 3 (5.18 KB, patch)
2007-08-11 04:51 PDT, Andrew Wellington
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Olle Lundberg 2007-07-17 12:10:42 PDT
The spec says:
[...]
The name of the CSS property. See the CSS property index.
[...]
http://www.w3.org/TR/DOM-Level-2-Style/css.html#CSS-CSSStyleDeclaration-getPropertyValue

CSS properties are case insensitive, therefore the string passed to getPropertyValue should also be case insensitive. In webkit this is not the case.

Test case: http://testserver.se/webkit/tests/getPropertyValue.html
Comment 1 Olle Lundberg 2007-07-17 12:11:58 PDT
Created attachment 15547 [details]
Test case
Comment 2 Alexey Proskuryakov 2007-07-18 02:46:03 PDT
Same problem with other CSSStyleDeclaration methods, e.g. removeProperty().
Comment 3 Andrew Wellington 2007-08-06 06:26:56 PDT
Created attachment 15847 [details]
Proposed patch

This should correct not only the getPropertyValue issue from the original report, but the other routines that Alexey noted.
Comment 4 Mark Rowe (bdash) 2007-08-07 13:57:08 PDT
Comment on attachment 15847 [details]
Proposed patch

Can you not use tolower rather than the manual lowercasing?
Comment 5 Sam Weinig 2007-08-08 20:00:21 PDT
The test case should really cover more cases (the ones Alexey noted) and can probably be dumpAsText().
Comment 6 Sam Weinig 2007-08-08 20:01:05 PDT
Comment on attachment 15847 [details]
Proposed patch

Marking r- until the above comments have been addressed.
Comment 7 Andrew Wellington 2007-08-11 04:37:25 PDT
Created attachment 15927 [details]
Propose patch 2

Use tolower, can't think why I didn't the first time... hrmm :-)

Better layout test that tests getPropertyValue, setProperty and removeProperty.
Comment 8 Mark Rowe (bdash) 2007-08-11 04:48:18 PDT
Comment on attachment 15927 [details]
Propose patch 2

r=me.  I think it would be worth adding the single line needed to dump the results as text as it would make the test more portable to other platforms.
Comment 9 Andrew Wellington 2007-08-11 04:51:14 PDT
Created attachment 15928 [details]
Proposed patch 3

dumpAsText version of layout test
Comment 10 Andrew Wellington 2007-08-11 04:54:16 PDT
Comment on attachment 15928 [details]
Proposed patch 3

Will commit dumpAsText version as in Mark's comment
Comment 11 Andrew Wellington 2007-08-11 04:59:03 PDT
Landed in r25008
Comment 12 Mark Rowe (bdash) 2007-08-16 04:56:19 PDT
<rdar://problem/5415308>