Bug 14645

Summary:

getPropertyValue should be case insensitive

Product:

WebKit

Reporter:

Olle Lundberg <olle.lundberg>

Component:

CSS

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ap, sam, zcorpan

Priority:

Keywords:

InRadar

Version:

523.x (Safari 3)

Hardware:

Mac

OS:

OS X 10.4

URL:

http://testserver.se/webkit/tests/getPropertyValue.html

Attachments:

Description	Flags
Test case	none
Proposed patch	sam: review-
Propose patch 2	mrowe: review+
Proposed patch 3	none

Olle Lundberg

Reported 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

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
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Olle Lundberg

Comment 1 2007-07-17 12:11:58 PDT

Created attachment 15547 [details] Test case

Alexey Proskuryakov

Comment 2 2007-07-18 02:46:03 PDT

Same problem with other CSSStyleDeclaration methods, e.g. removeProperty().

Andrew Wellington

Comment 3 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.

Mark Rowe (bdash)

Comment 4 2007-08-07 13:57:08 PDT

Comment on attachment 15847 [details] Proposed patch Can you not use tolower rather than the manual lowercasing?

Sam Weinig

Comment 5 2007-08-08 20:00:21 PDT

The test case should really cover more cases (the ones Alexey noted) and can probably be dumpAsText().

Sam Weinig

Comment 6 2007-08-08 20:01:05 PDT

Comment on attachment 15847 [details] Proposed patch Marking r- until the above comments have been addressed.

Andrew Wellington

Comment 7 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.

Mark Rowe (bdash)

Comment 8 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.

Andrew Wellington

Comment 9 2007-08-11 04:51:14 PDT

Created attachment 15928 [details] Proposed patch 3 dumpAsText version of layout test

Andrew Wellington

Comment 10 2007-08-11 04:54:16 PDT

Comment on attachment 15928 [details] Proposed patch 3 Will commit dumpAsText version as in Mark's comment

Andrew Wellington

Comment 11 2007-08-11 04:59:03 PDT

Landed in r25008

Mark Rowe (bdash)

Comment 12 2007-08-16 04:56:19 PDT

<rdar://problem/5415308>

Note You need to log in before you can comment on or make changes to this bug.