CSSStyleDeclaration.cssFloat is not enumerated property

CSSStyleDeclaration.cssFloat is not enumerated property

Whenver we try to get the value of the property cssFloat (style.cssFloat),
it is found and gets evaluated to none, however it is not part of the
enumerated properties of CSSStyleDeclaration. This patch changes the same.

It also fixes the below:
Blink and Webkit supports non-standard behaviour e.g.
getComputedStyle(e).cssBoxShadow. It's suspected that a very few
(or none at all) use this. Earlier we had added an useCounter for all
non-standard cssX queries [[https://codereview.chromium.org/621283002]
but now one of the cases getComputedStyle(e).cssFloat is being specced,
so we should really only be counting when it isn't cssFloat. This CL
modifies the same.

BUG=492999, 413205

[Sunil Ratnu, 2015-04-30 12:15:11]

sunil.ratnu@samsung.com changed reviewers:
+ haraken@chromium.org, timloh@chromium.org, vivekg@chromium.org

[Sunil Ratnu, 2015-04-30 12:15:11]

Please have a look.

[Timothy Loh, 2015-05-04 01:11:32]

This is probably better if we instead just add a cssFloat to the idl (similar to
what the spec says http://dev.w3.org/csswg/cssom). Then I think we won't get to
this code at all in those cases.

[Sunil Ratnu, 2015-05-05 08:43:09]

On 2015/05/04 01:11:32, Timothy Loh wrote:
> This is probably better if we instead just add a cssFloat to the idl (similar
to
> what the spec says http://dev.w3.org/csswg/cssom). Then I think we won't get
to
> this code at all in those cases.

Hi Timothy,

Thanks for your response. If I understood you correctly, you meant using
"[TreatNullAs=EmptyString] attribute DOMString cssFloat" in the
CSSStyleDeclaration interface file? If that is the case, then we need to define
setters and getters for this attribute else the build would break.

Thanks & Regards,
Sunil

[Timothy Loh, 2015-05-06 01:15:18]

On 2015/05/05 08:43:09, Sunil Ratnu wrote:
> On 2015/05/04 01:11:32, Timothy Loh wrote:
> > This is probably better if we instead just add a cssFloat to the idl
(similar
> to
> > what the spec says http://dev.w3.org/csswg/cssom). Then I think we won't get
> to
> > this code at all in those cases.
> 
> Hi Timothy,
> 
> Thanks for your response. If I understood you correctly, you meant using
> "[TreatNullAs=EmptyString] attribute DOMString cssFloat" in the
> CSSStyleDeclaration interface file? If that is the case, then we need to
define
> setters and getters for this attribute else the build would break.
> 
> Thanks & Regards,
> Sunil

Yep, this seems reasonable to me.

[Sunil Ratnu, 2015-06-23 13:00:23]

Hi Timothy,

Apologies for being late while following up with the review as I was a bit busy
with other internal work here. I've made the changes pointed out by you. Also
I've linked the other bug which this patch solves (updated the description too).
Could you please take a look at the updated patch set? Test case addition is
what remains, I will add that pretty soon.

Here is the spec part for cssFloat:
http://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-cssfloat

Thanks & Regards,
Sunil

[Timothy Loh, 2015-06-24 00:35:00]

Thanks for getting back to this.

https://codereview.chromium.org/1113013002/diff/20001/Source/core/css/CSSComp...
File Source/core/css/CSSComputedStyleDeclaration.cpp (right):

https://codereview.chromium.org/1113013002/diff/20001/Source/core/css/CSSComp...
Source/core/css/CSSComputedStyleDeclaration.cpp:408: return "";
These are wrong, the spec defines cssFloat in terms of getPropertyValue and
setProperty, so this should probably return an actual value and setCSSFloat
should probably raise an exception. Maybe we can make the base class implement
these to call the virtual functions instead?

http://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-cssfloat

[Sunil Ratnu, 2015-06-24 13:17:11]

Hi Timothy,

Thanks for the review. I have added a test case for this. Please review the
latest patchset.

Thanks & Regards,
Sunil

https://codereview.chromium.org/1113013002/diff/20001/Source/core/css/CSSComp...
File Source/core/css/CSSComputedStyleDeclaration.cpp (right):

https://codereview.chromium.org/1113013002/diff/20001/Source/core/css/CSSComp...
Source/core/css/CSSComputedStyleDeclaration.cpp:408: return "";
Using getPropertyValue(CSSPropertyFloat) in the cssFloat() method now.

Also to throw the exception, we will need to use [RaisesException=Setter] in the
idl. The spec says that "Any exceptions thrown must be re-thrown", could you
please let me know what kind of exception type are we supposed to throw here?

In the base class almost all the methods have been kept as pure virtual and have
their definitions in the derived classes, so it seems better to follow the same
here as well.

On 2015/06/24 00:35:00, Timothy Loh wrote:
> These are wrong, the spec defines cssFloat in terms of getPropertyValue and
> setProperty, so this should probably return an actual value and setCSSFloat
> should probably raise an exception. Maybe we can make the base class implement
> these to call the virtual functions instead?
> 
> http://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-cssfloat

[Sunil Ratnu, 2015-06-24 17:14:15]

On 2015/06/24 13:17:11, Sunil Ratnu wrote:
> Hi Timothy,
> 
> Thanks for the review. I have added a test case for this. Please review the
> latest patchset.
> 
> Thanks & Regards,
> Sunil
> 
>
https://codereview.chromium.org/1113013002/diff/20001/Source/core/css/CSSComp...
> File Source/core/css/CSSComputedStyleDeclaration.cpp (right):
> 
>
https://codereview.chromium.org/1113013002/diff/20001/Source/core/css/CSSComp...
> Source/core/css/CSSComputedStyleDeclaration.cpp:408: return "";
> Using getPropertyValue(CSSPropertyFloat) in the cssFloat() method now.
> 
> Also to throw the exception, we will need to use [RaisesException=Setter] in
the
> idl. The spec says that "Any exceptions thrown must be re-thrown", could you
> please let me know what kind of exception type are we supposed to throw here?
> 
> In the base class almost all the methods have been kept as pure virtual and
have
> their definitions in the derived classes, so it seems better to follow the
same
> here as well.
> 
> On 2015/06/24 00:35:00, Timothy Loh wrote:
> > These are wrong, the spec defines cssFloat in terms of getPropertyValue and
> > setProperty, so this should probably return an actual value and setCSSFloat
> > should probably raise an exception. Maybe we can make the base class
implement
> > these to call the virtual functions instead?
> > 
> > http://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-cssfloat

Oops... The failure does look real here. :/

[Timothy Loh, 2015-10-06 03:53:15]

Closing this since a similar fix was landed in
https://codereview.chromium.org/1288623005