Ensure we never remove the style attribute when syncing it from CSSOM.

Ensure we never remove the style attribute when syncing it from CSSOM.

This makes us align with other browsers when, editing the style attribute, and
also with the spec, per[1]:

> Mutating the declarations must set the style content attribute on the context
> object to the serialization of the declarations

See also [2].

Note that this behavior also affects editing. I've just updated the editing
tests.

[1]: https://drafts.csswg.org/cssom/#the-elementcssinlinestyle-interface
[2]: https://github.com/whatwg/html/issues/2306

BUG=686686
Review-Url: https://codereview.chromium.org/2800723002
Cr-Commit-Position: refs/heads/master@{#465181}
Committed: https://chromium.googlesource.com/chromium/src/+/4e41fe9e3515a33dc5482e2d628456beb4bd65ef

[emilio, 2017-04-05 14:06:30]

The CQ bit was checked by ecobos@igalia.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-05 14:06:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-05 15:31:50]

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

[commit-bot: I haz the power, 2017-04-05 15:31:51]

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

[emilio, 2017-04-05 20:07:09]

The CQ bit was checked by ecobos@igalia.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-05 20:07:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-05 21:20:00]

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

[commit-bot: I haz the power, 2017-04-05 21:20:01]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[emilio, 2017-04-06 00:13:42]

The CQ bit was checked by ecobos@igalia.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-06 00:14:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[emilio, 2017-04-06 00:17:03]

Description was changed from

==========
Set the empty style attribute to the empty string when there's no style
declaration instead of removing it.

BUG=686686
==========

to

==========
Ensure we never remove the style attribute when syncing it from CSSOM.

BUG=686686
==========

[emilio, 2017-04-06 00:19:51]

ecobos@igalia.com changed reviewers:
+ esprehn@chromium.org

[emilio, 2017-04-06 00:19:51]

Hi esprehn@,

I've put this up to attempt to fix issue 686686. I wonder if you think this is a
reasonable solution? This should prevent us incorrectly deleting the style
attribute from CSSOM in all cases, without preventing the inline declaration
optimization, nor swapping it with an immutable one.

If you think so, I still have a few editing/ test expectations to update, but
feedback would be appreciated before continuing that task.

Thanks in advance!

[esprehn, 2017-04-06 00:25:38]

What do other browsers do? What does the spec say? As an author I guess I'd be
surprised if I removed every property that the element is left with a style=""
in the serialization. Does the spec really want that?

[emilio, 2017-04-06 01:04:46]

On 2017/04/06 00:25:38, esprehn wrote:
> What do other browsers do? What does the spec say? As an author I guess I'd be
> surprised if I removed every property that the element is left with a style=""
> in the serialization. Does the spec really want that?

Yes, and every other browser does that. The spec seems clear, see
https://github.com/whatwg/html/issues/2306#issuecomment-276035317:

> Mutating the declarations must set the style content attribute on the context
object to the serialization of the declarations.

[esprehn, 2017-04-07 00:04:57]

esprehn@chromium.org changed reviewers:
+ meade@chromium.org, nainar@chromium.org

[esprehn, 2017-04-07 00:04:58]

Your bots are red btw, meade@ or nainar@ should look at the patch too to get
familiar with this system too.

[nainar, 2017-04-07 00:47:21]

On 2017/04/07 at 00:04:58, esprehn wrote:
> Your bots are red btw, meade@ or nainar@ should look at the patch too to get
familiar with this system too.

Thanks for looping us in esprehn@! 

So looking at the change it makes sense specially since that is how I read the
spec too and the other browsers are in unison on that implementation. 

Doesn't mean it isn't icky though for the same case that esprehn mentioned in
comment 14. Having an empty style should mean that I get null not "".

[emilio, 2017-04-08 23:55:48]

The CQ bit was checked by ecobos@igalia.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-08 23:56:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-09 01:16:01]

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

[commit-bot: I haz the power, 2017-04-09 01:16:02]

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

[emilio, 2017-04-09 03:55:21]

The CQ bit was checked by ecobos@igalia.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-09 03:55:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-09 05:43:11]

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

[commit-bot: I haz the power, 2017-04-09 05:43:12]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[meade_UTC10, 2017-04-11 05:05:01]

Also, could you please update your CL title and description with why this needs
doing? Your CL title is currently cut off and ends with "...".

[emilio, 2017-04-17 11:33:29]

The CQ bit was checked by ecobos@igalia.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-17 11:33:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[emilio, 2017-04-17 11:37:47]

Description was changed from

==========
Ensure we never remove the style attribute when syncing it from CSSOM.

BUG=686686
==========

to

==========
Ensure we never remove the style attribute when syncing it from CSSOM.

This makes us align with other browsers when, editing the style attribute, and
also with the spec, per[1]:

> Mutating the declarations must set the style content attribute on the context
> object to the serialization of the declarations

See also [2].

Note that this behavior also affects editing. I've just updated the editing
tests.

[1]: https://drafts.csswg.org/cssom/#the-elementcssinlinestyle-interface
[2]: https://github.com/whatwg/html/issues/2306

BUG=686686
==========

[emilio, 2017-04-17 11:38:10]

Description was changed from

==========
Ensure we never remove the style attribute when syncing it from CSSOM.

This makes us align with other browsers when, editing the style attribute, and
also with the spec, per[1]:

> Mutating the declarations must set the style content attribute on the context
> object to the serialization of the declarations

See also [2].

Note that this behavior also affects editing. I've just updated the editing
tests.

[1]: https://drafts.csswg.org/cssom/#the-elementcssinlinestyle-interface
[2]: https://github.com/whatwg/html/issues/2306

BUG=686686
==========

to

==========
Ensure we never remove the style attribute when syncing it from CSSOM.

This makes us align with other browsers when, editing the style attribute, and
also with the spec, per[1]:

> Mutating the declarations must set the style content attribute on the context
> object to the serialization of the declarations

See also [2].

Note that this behavior also affects editing. I've just updated the editing
tests.

[1]: https://drafts.csswg.org/cssom/#the-elementcssinlinestyle-interface
[2]: https://github.com/whatwg/html/issues/2306

BUG=686686
==========

[emilio, 2017-04-17 11:40:52]

On 2017/04/11 05:05:01, Eddy_UTC10 wrote:
> Also, could you please update your CL title and description with why this
needs
> doing? Your CL title is currently cut off and ends with "...".

Yup, done, thanks for the suggestion.

Sorry for the lag here, was away last week. Bots should be green now, so if
someone could review it, it'd be appreciated.

Thanks!

[commit-bot: I haz the power, 2017-04-17 13:17:46]

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

[commit-bot: I haz the power, 2017-04-17 13:17:47]

Dry run: This issue passed the CQ dry run.

[nainar, 2017-04-18 00:34:04]

lgtm

[emilio, 2017-04-18 00:43:33]

The CQ bit was checked by ecobos@igalia.com

[emilio, 2017-04-18 00:44:11]

On 2017/04/18 00:34:04, nainar wrote:
> lgtm

Thanks for the review! :)

[commit-bot: I haz the power, 2017-04-18 00:44:11]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-18 00:53:37]

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

[commit-bot: I haz the power, 2017-04-18 00:53:39]

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

[nainar, 2017-04-18 01:03:02]

On 2017/04/18 at 00:53:39, commit-bot wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

Yeah I am not OWNER. Should probably wait on meade@ or esprehn@ for that

[esprehn, 2017-04-18 01:57:01]

lgtm w/ nit.

https://codereview.chromium.org/2800723002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/Element.cpp (right):

https://codereview.chromium.org/2800723002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/Element.cpp:4018: styleAttr,
AtomicString(inline_style ? inline_style->AsText() : ""));
inline_style ? AtomicString(inline_style->AsText()) : g_empty_atom

[emilio, 2017-04-18 07:22:17]

The CQ bit was checked by ecobos@igalia.com

[emilio, 2017-04-18 07:22:18]

The patchset sent to the CQ was uploaded after l-g-t-m from nainar@chromium.org,
esprehn@chromium.org
Link to the patchset: https://codereview.chromium.org/2800723002/#ps130001
(title: "Nits")

[commit-bot: I haz the power, 2017-04-18 07:22:37]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-18 08:57:31]

CQ is committing da patch.

Bot data: {"patchset_id": 130001, "attempt_start_ts": 1492500137449380,
"parent_rev": "7e377438d1c86ec06009e10d24a88ea84342e594", "commit_rev":
"4e41fe9e3515a33dc5482e2d628456beb4bd65ef"}

[commit-bot: I haz the power, 2017-04-18 08:57:44]

Description was changed from

==========
Ensure we never remove the style attribute when syncing it from CSSOM.

This makes us align with other browsers when, editing the style attribute, and
also with the spec, per[1]:

> Mutating the declarations must set the style content attribute on the context
> object to the serialization of the declarations

See also [2].

Note that this behavior also affects editing. I've just updated the editing
tests.

[1]: https://drafts.csswg.org/cssom/#the-elementcssinlinestyle-interface
[2]: https://github.com/whatwg/html/issues/2306

BUG=686686
==========

to

==========
Ensure we never remove the style attribute when syncing it from CSSOM.

This makes us align with other browsers when, editing the style attribute, and
also with the spec, per[1]:

> Mutating the declarations must set the style content attribute on the context
> object to the serialization of the declarations

See also [2].

Note that this behavior also affects editing. I've just updated the editing
tests.

[1]: https://drafts.csswg.org/cssom/#the-elementcssinlinestyle-interface
[2]: https://github.com/whatwg/html/issues/2306

BUG=686686

Review-Url: https://codereview.chromium.org/2800723002
Cr-Commit-Position: refs/heads/master@{#465181}
Committed:
https://chromium.googlesource.com/chromium/src/+/4e41fe9e3515a33dc5482e2d6284...
==========

[commit-bot: I haz the power, 2017-04-18 08:57:45]

Committed patchset #8 (id:130001) as
https://chromium.googlesource.com/chromium/src/+/4e41fe9e3515a33dc5482e2d6284...