Forward assigning to style on HTMLElement, SVGElement and CSSStyleRule to style.cssText

Forward assigning to style on HTMLElement, SVGElement and CSSStyleRule to style.cssText

Currently setting element.style doesn't update the inline style of that
element. This is inconsistent with CSSOM spec ->
https://drafts.csswg.org/cssom/#cssstylerule. This patch forwards style
assignment on HTMLElement, SVGElement and CSSStyleRule to style.cssText
by changing the relevant idl files. The patch ensures that if the style
attribute is set, changed or removed, the declarations must be
updated as appropriate. FF supports forwarding assigning to style on
HTMLElement and SVGElement but not on CSSStyleRule. IE doesn't support
forwarding assignment to style on either of the three.

This patch also changes the layout tests in order to reflect the patch.
The expectation for checking in an input element's width and height
attributes without a loaded image has also been changed to a failing
expectation. This is as setting the display to none causes the
element and its descendants to not generate boxes hence returning a
width of 0.

BUG=533691
Committed: https://crrev.com/d1a7b0cbd0a511459f0f6202dfe61e27aa47df46
Cr-Commit-Position: refs/heads/master@{#353235}

[nainar, 2015-10-06 03:25:31]

nainar@chromium.org changed reviewers:
+ alancutter@chromium.org

[nainar, 2015-10-06 03:25:31]

Alan,

PTAL?

Thanks!

[nainar, 2015-10-06 05:52:03]

nainar@chromium.org changed reviewers:
+ timloh@chromium.org

[rune, 2015-10-06 08:19:52]

rune@opera.com changed reviewers:
+ rune@opera.com

[rune, 2015-10-06 08:19:53]

https://codereview.chromium.org/1384973004/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/platform/linux/fast/css/setting-style-attribute-update-inline-style-expected.txt
(right):

https://codereview.chromium.org/1384973004/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/platform/linux/fast/css/setting-style-attribute-update-inline-style-expected.txt:1:
layer at (0,0) size 800x600
Please use text-based tests if possible.

https://codereview.chromium.org/1384973004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSStyleRule.idl (right):

https://codereview.chromium.org/1384973004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSStyleRule.idl:25: [SameObject,
PutForwards=cssText] readonly attribute CSSStyleDeclaration style;
This doesn't work in Gecko when I tested here. Are you sure this shouldn't be
limited to Element.style? (edit: I see that the current CSSOM draft forwards
style to style.cssText for all style members.

If not, it should also be backed up by a test.

The CL description should also reflect that we're sync'ing with the CSSOM draft,
not FF.

https://codereview.chromium.org/1384973004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/forms/ImageInputType.cpp (left):

https://codereview.chromium.org/1384973004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/forms/ImageInputType.cpp:200: if
(!element->layoutObject()) {
The changes in this file look unrelated to me.

[nainar, 2015-10-06 23:52:52]

Hi,

PTAL?

Thanks!

https://codereview.chromium.org/1384973004/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/platform/linux/fast/css/setting-style-attribute-update-inline-style-expected.txt
(right):

https://codereview.chromium.org/1384973004/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/platform/linux/fast/css/setting-style-attribute-update-inline-style-expected.txt:1:
layer at (0,0) size 800x600
Done.

https://codereview.chromium.org/1384973004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSStyleRule.idl (right):

https://codereview.chromium.org/1384973004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSStyleRule.idl:25: [SameObject,
PutForwards=cssText] readonly attribute CSSStyleDeclaration style;
Done.

https://codereview.chromium.org/1384973004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/forms/ImageInputType.cpp (left):

https://codereview.chromium.org/1384973004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/forms/ImageInputType.cpp:200: if
(!element->layoutObject()) {
This statement checks to see if element->layoutObject() doesn't exist but then
proceeds to use it if the image is available as you can see in the nested if.

Also, this condition was causing tests to break when included with the patch.

[Timothy Loh, 2015-10-07 00:03:01]

https://codereview.chromium.org/1384973004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/forms/ImageInputType.cpp (left):

https://codereview.chromium.org/1384973004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/forms/ImageInputType.cpp:200: if
(!element->layoutObject()) {
On 2015/10/06 23:52:52, nainar wrote:
> This statement checks to see if element->layoutObject() doesn't exist but then
> proceeds to use it if the image is available as you can see in the nested if.
> 
> Also, this condition was causing tests to break when included with the patch.

What tests?

[nainar, 2015-10-07 00:08:42]

On 2015/10/07 at 00:03:01, timloh wrote:
>
https://codereview.chromium.org/1384973004/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/html/forms/ImageInputType.cpp (left):
> 
>
https://codereview.chromium.org/1384973004/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/html/forms/ImageInputType.cpp:200: if
(!element->layoutObject()) {
> On 2015/10/06 23:52:52, nainar wrote:
> > This statement checks to see if element->layoutObject() doesn't exist but
then
> > proceeds to use it if the image is available as you can see in the nested
if.
> > 
> > Also, this condition was causing tests to break when included with the
patch.
> 
> What tests?

input-width-height-attributes-without-renderer-loaded-image.html and
input-width-height-attributes-without-renderer.html -> the element.width and
element.height were returning incorrect values in both cases

[Timothy Loh, 2015-10-07 00:31:33]

On 2015/10/07 00:08:42, nainar wrote:
> On 2015/10/07 at 00:03:01, timloh wrote:
> >
>
https://codereview.chromium.org/1384973004/diff/20001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/core/html/forms/ImageInputType.cpp (left):
> > 
> >
>
https://codereview.chromium.org/1384973004/diff/20001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/html/forms/ImageInputType.cpp:200: if
> (!element->layoutObject()) {
> > On 2015/10/06 23:52:52, nainar wrote:
> > > This statement checks to see if element->layoutObject() doesn't exist but
> then
> > > proceeds to use it if the image is available as you can see in the nested
> if.
> > > 
> > > Also, this condition was causing tests to break when included with the
> patch.
> > 
> > What tests?
> 
> input-width-height-attributes-without-renderer-loaded-image.html and
> input-width-height-attributes-without-renderer.html -> the element.width and
> element.height were returning incorrect values in both cases

It's still a separate change, should fix it separately

[nainar, 2015-10-08 00:59:48]

Hi,

PTAL?

Thanks!

[nainar, 2015-10-08 02:47:27]

Tim,

PTAL?

Thanks!

[nainar, 2015-10-08 04:04:35]

Hi,

PTAL?

Thanks!

[Timothy Loh, 2015-10-08 05:30:28]

On 2015/10/08 04:04:35, nainar wrote:
> Hi,
> 
> PTAL?
> 
> Thanks!

lgtm

[nainar, 2015-10-08 05:36:31]

The CQ bit was checked by nainar@chromium.org

[commit-bot: I haz the power, 2015-10-08 05:37:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1384973004/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1384973004/220001

[commit-bot: I haz the power, 2015-10-08 06:53:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-08 06:53:50]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[nainar, 2015-10-08 07:18:58]

The CQ bit was checked by nainar@chromium.org

[commit-bot: I haz the power, 2015-10-08 07:19:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1384973004/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1384973004/220001

[rune, 2015-10-08 07:23:50]

https://codereview.chromium.org/1384973004/diff/220001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/forms/input-width-height-attributes-without-renderer-loaded-image-expected.txt
(right):

https://codereview.chromium.org/1384973004/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/forms/input-width-height-attributes-without-renderer-loaded-image-expected.txt:1:
FAIL e.width should be 16. Was 0.
Why did this result change with your patch? I don't see style being set in that
test.

[rune, 2015-10-08 07:28:28]

https://codereview.chromium.org/1384973004/diff/220001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/forms/input-width-height-attributes-without-renderer-loaded-image-expected.txt
(right):

https://codereview.chromium.org/1384973004/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/forms/input-width-height-attributes-without-renderer-loaded-image-expected.txt:1:
FAIL e.width should be 16. Was 0.
On 2015/10/08 07:23:50, rune wrote:
> Why did this result change with your patch? I don't see style being set in
that
> test.

Also, if you think the new result is correct, you should update the test to
reflect that expecting 0 which should print a PASS.

[rune, 2015-10-08 07:37:42]

https://codereview.chromium.org/1384973004/diff/220001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/forms/input-width-height-attributes-without-renderer-loaded-image-expected.txt
(right):

https://codereview.chromium.org/1384973004/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/forms/input-width-height-attributes-without-renderer-loaded-image-expected.txt:1:
FAIL e.width should be 16. Was 0.
On 2015/10/08 07:28:28, rune wrote:
> On 2015/10/08 07:23:50, rune wrote:
> > Why did this result change with your patch? I don't see style being set in
> that
> > test.
> 
> Also, if you think the new result is correct, you should update the test to
> reflect that expecting 0 which should print a PASS.

I was looking at the not-loaded case. I see now why it changed. The test didn't
do what it said originally, since display:none was never set. You should make
this a PASS, though.

[commit-bot: I haz the power, 2015-10-08 08:36:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-08 08:36:04]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[nainar, 2015-10-08 22:22:43]

https://codereview.chromium.org/1384973004/diff/220001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/forms/input-width-height-attributes-without-renderer-loaded-image-expected.txt
(right):

https://codereview.chromium.org/1384973004/diff/220001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/forms/input-width-height-attributes-without-renderer-loaded-image-expected.txt:1:
FAIL e.width should be 16. Was 0.
Done.

[rune, 2015-10-08 22:39:44]

lgtm

[Timothy Loh, 2015-10-08 22:44:51]

On 2015/10/08 22:22:43, nainar wrote:
>
https://codereview.chromium.org/1384973004/diff/220001/third_party/WebKit/Lay...
> File
>
third_party/WebKit/LayoutTests/fast/forms/input-width-height-attributes-without-renderer-loaded-image-expected.txt
> (right):
> 
>
https://codereview.chromium.org/1384973004/diff/220001/third_party/WebKit/Lay...
>
third_party/WebKit/LayoutTests/fast/forms/input-width-height-attributes-without-renderer-loaded-image-expected.txt:1:
> FAIL e.width should be 16. Was 0.
> Done.

Is 0 the correct thing here though? FF passes the test as is (and already
forwards setting style).

[nainar, 2015-10-09 04:36:45]

Retaining the test expectation as a FAIL, as it works in FF and the expected
resulting with forwarding setting a style is a failure.

[nainar, 2015-10-09 04:36:54]

The CQ bit was checked by nainar@chromium.org

[nainar, 2015-10-09 04:36:55]

The patchset sent to the CQ was uploaded after l-g-t-m from timloh@chromium.org,
rune@opera.com
Link to the patchset: https://codereview.chromium.org/1384973004/#ps260001
(title: "Final patch")

[commit-bot: I haz the power, 2015-10-09 04:37:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1384973004/260001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1384973004/260001

[commit-bot: I haz the power, 2015-10-09 06:15:09]

Committed patchset #14 (id:260001)

[commit-bot: I haz the power, 2015-10-09 06:16:15]

Patchset 14 (id:??) landed as
https://crrev.com/d1a7b0cbd0a511459f0f6202dfe61e27aa47df46
Cr-Commit-Position: refs/heads/master@{#353235}