Bug 14645 - getPropertyValue should be case insensitive
Summary: getPropertyValue should be case insensitive
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:
Reported: 2007-07-17 12:10 PDT by Olle Lundberg
Modified: 2007-08-16 04:56 PDT (History)
3 users (show)

See Also:

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.

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