CSSStyleDeclaration.getPropertyValue returns empty string instead of null. (issue 11047008)

[efor...@google.com]

Reviewers: vsm, blois,

Message:
The corresponding change for Dartium is being tracked here:
http://code.google.com/p/dart/issues/detail?id=5617

Description:
CSSStyleDeclaration.getPropertyValue returns empty string instead of null.

Please review this at https://codereview.chromium.org/11047008/

SVN Base: http://dart.googlecode.com/svn/branches/bleeding_edge/dart/

Affected files:
M lib/html/dart2js/html_dart2js.dart
M lib/html/scripts/systemhtml.py
M lib/html/templates/html/impl/impl_CSSStyleDeclaration.darttemplate
M tests/html/cssstyledeclaration_test.dart
M tests/html/html.status


Index: lib/html/dart2js/html_dart2js.dart
===================================================================
--- lib/html/dart2js/html_dart2js.dart (revision 13149)
+++ lib/html/dart2js/html_dart2js.dart (working copy)
@@ -4334,8 +4334,6 @@

String getPropertyShorthand(String propertyName) native;

- String getPropertyValue(String propertyName) native;
-
bool isPropertyImplicit(String propertyName) native;

String item(int index) native;
@@ -4343,6 +4341,14 @@
String removeProperty(String propertyName) native;


+ String getPropertyValue(String propertyName) {
+ if (JS('bool', '!#.getPropertyValue(#)', this, propertyName)) {
+ return '';
+ } else {
+ return JS('String', '#.getPropertyValue(#)', this, propertyName);
+ }
+ }
+
void setProperty(String propertyName, String value, [String priority])
native '''
this.setProperty(propertyName, value, priority);
// Bug #2772, IE9 requires a poke to actually apply the value.
Index: lib/html/scripts/systemhtml.py
===================================================================
--- lib/html/scripts/systemhtml.py (revision 13149)
+++ lib/html/scripts/systemhtml.py (working copy)
@@ -11,6 +11,7 @@
from systembase import *

_js_custom_members = set([
+ 'CSSStyleDeclaration.getPropertyValue',
'CSSStyleDeclaration.setProperty',
'Element.insertAdjacentElement',
'Element.insertAdjacentHTML',
Index: lib/html/templates/html/impl/impl_CSSStyleDeclaration.darttemplate
===================================================================
--- lib/html/templates/html/impl/impl_CSSStyleDeclaration.darttemplate
(revision 13149)
+++ lib/html/templates/html/impl/impl_CSSStyleDeclaration.darttemplate
(working copy)
@@ -24,6 +24,14 @@
$!MEMBERS

$if DART2JS
+ String getPropertyValue(String propertyName) {
+ if (JS('bool', '!#.getPropertyValue(#)', this, propertyName)) {
+ return '';
+ } else {
+ return JS('String', '#.getPropertyValue(#)', this, propertyName);
+ }
+ }
+
void setProperty(String propertyName, String value, [String priority])
native '''
this.setProperty(propertyName, value, priority);
// Bug #2772, IE9 requires a poke to actually apply the value.
Index: tests/html/cssstyledeclaration_test.dart
===================================================================
--- tests/html/cssstyledeclaration_test.dart (revision 13149)
+++ tests/html/cssstyledeclaration_test.dart (working copy)
@@ -46,6 +46,15 @@
equals("color: blue;"));
});

+ test('CSS property empty getters and setters', () {
+ var style = createTestStyle();
+ expect(style.border, equals(""));
+
+ style.border = "1px solid blue";
+ style.border = "";
+ expect(style.border, equals(""));
+ });
+
test('CSS property getters and setters', () {
var style = createTestStyle();
expect(style.color, equals("blue"));
Index: tests/html/html.status
===================================================================
--- tests/html/html.status (revision 13149)
+++ tests/html/html.status (working copy)
@@ -16,6 +16,9 @@
xhr_cross_origin_test: Skip # Issue 4502: Passes with
--allow-external-pages.

[ $compiler == none && ($runtime == drt || $runtime == dartium) ]
+cssstyledeclaration_test: Fail # Issue 5617: Need to make corresponding
dartium change.
+
+[ $compiler == none && ($runtime == drt || $runtime == dartium) ]
request_animation_frame_test: Skip # drt hangs; requestAnimationFrame
not implemented

[ $compiler == none && $runtime == drt && $system == windows]

[bl...@google.com]

https://codereview.chromium.org/11047008/diff/3001/lib/html/dart2js/html_dart2js.dart
File lib/html/dart2js/html_dart2js.dart (right):

https://codereview.chromium.org/11047008/diff/3001/lib/html/dart2js/html_dart2js.dart#newcode4345
lib/html/dart2js/html_dart2js.dart:4345: + if (JS('bool',
'!#.getPropertyValue(#)', this, propertyName)) {
This is getting the property value twice- should probably get it once,
check if it's null and return empty string if so.

https://codereview.chromium.org/11047008/

[efor...@google.com]

Fixed up. PTAL

https://codereview.chromium.org/11047008/

[bl...@google.com]

lgtm



https://codereview.chromium.org/11047008/