Pseudo and non pseudo elements should return correct computed content value

Pseudo and non pseudo elements should return correct computed content value

This patch makes sure that content computes to "normal" for elements
(not pseudo elements) and to "none" if the specified value of pseudo
elements ::before and ::after is "normal."

This patch makes us interoperable with all other browsers for the
case of pseudo elements :before and ::after where the specified value
is "normal".

With regards to non psuedo elements we are now interoperable with IE
and the spec and return "normal" for the computed content all all non
psuedo elements.
Other browsers are inconcistent on this behaviour:
1. Edge returns an empty string for non pseudo elements
2. FF returns none for non pseudo elements.

The spec implemented is given here:
https://drafts.csswg.org/css2/generate.html#content

BUG=597500
Committed: https://crrev.com/aaeefae6c3a945e7fcfef382c3a82002ffae0e19
Cr-Commit-Position: refs/heads/master@{#383949}

[nainar, 2016-03-17 06:09:45]

Description was changed from

==========
Pseudo and non pseudo elements should return correct computed content value

This patch makes sure that content computes to "normal" for elements
(not pseudo elements) and to "none" if the specified value of pseudo
elements is "normal."

This is interoperable with IE, Edge returns an empty string for non
pseudo elements while FF returns none.

BUG=313670
==========

to

==========
Pseudo and non pseudo elements should return correct computed content value

This patch makes sure that content computes to "normal" for elements
(not pseudo elements) and to "none" if the specified value of pseudo
elements is "normal."

This is interoperable with IE, Edge returns an empty string for non
pseudo elements while FF returns none.

The spec is given here: https://www.w3.org/TR/CSS2/generate.html#content

BUG=313670
==========

[nainar, 2016-03-17 06:09:45]

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

[nainar, 2016-03-17 06:22:00]

@alancutter, 

PTAL? 

Thanks!

[alancutter (OOO until 2018), 2016-03-17 06:40:24]

Can we assign these values on the ComputedStyle directly? Switching things
around in getComputedStyle() doesn't exactly follow the CSS model.

https://codereview.chromium.org/1812763002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp
(right):

https://codereview.chromium.org/1812763002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp:2386: return
cssValuePool().createIdentifierValue(CSSValueNormal);
Should this be guarded by !style.hasContent()?

[nainar, 2016-03-17 06:54:24]

From what I can see ComputedStyle stores content information on ContentData
which stores information about Counters, Quotes, Text and StyleImage. I could
either expand the class to contain information about CSSValue or return the
value here in getComputedStyle. I chose  to return the value in
getComputedStyle, let me know if you still want me to change it.

https://codereview.chromium.org/1812763002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp
(right):

https://codereview.chromium.org/1812763002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp:2386: return
cssValuePool().createIdentifierValue(CSSValueNormal);
As far as I understand from https://www.w3.org/TR/css3-content/#content -> if
the non pseudo element doesn't have text then we should set it to normal. Am I
wrong here?

[alancutter (OOO until 2018), 2016-03-17 10:18:08]

On 2016/03/17 at 06:54:24, nainar wrote:
> From what I can see ComputedStyle stores content information on ContentData
which stores information about Counters, Quotes, Text and StyleImage. I could
either expand the class to contain information about CSSValue or return the
value here in getComputedStyle. I chose  to return the value in
getComputedStyle, let me know if you still want me to change it.

I mean the ComputedStyle equivalents of these values, not the CSSValue
representations.

> 
>
https://codereview.chromium.org/1812763002/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp
(right):
> 
>
https://codereview.chromium.org/1812763002/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp:2386:
return cssValuePool().createIdentifierValue(CSSValueNormal);
> As far as I understand from https://www.w3.org/TR/css3-content/#content -> if
the non pseudo element doesn't have text then we should set it to normal. Am I
wrong here?

This spec is different to the one linked in the description.

[nainar, 2016-03-17 23:46:53]

Description was changed from

==========
Pseudo and non pseudo elements should return correct computed content value

This patch makes sure that content computes to "normal" for elements
(not pseudo elements) and to "none" if the specified value of pseudo
elements is "normal."

This is interoperable with IE, Edge returns an empty string for non
pseudo elements while FF returns none.

The spec is given here: https://www.w3.org/TR/CSS2/generate.html#content

BUG=313670
==========

to

==========
Pseudo and non pseudo elements should return correct computed content value

This patch makes sure that content computes to "normal" for elements
(not pseudo elements) and to "none" if the specified value of pseudo
elements is "normal."

This is interoperable with IE, Edge returns an empty string for non
pseudo elements while FF returns none.

The spec is given here: 
https://drafts.csswg.org/css2/generate.html#content

BUG=313670
==========

[nainar, 2016-03-18 07:59:45]

Hi, 

I have updated the link to the spec in the description to the most updated one.
And have made the changes you asked for. PTAL? 

Thanks!

[alancutter (OOO until 2018), 2016-03-19 08:01:24]

https://codereview.chromium.org/1812763002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/style/ContentData.h (right):

https://codereview.chromium.org/1812763002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/style/ContentData.h:229: CSSValueID m_cssValueID;
How many different CSSValueIDs could be stored here? CSSValueIDs aren't
typically stored on ComputedStyle objects. Instead a separate enum is created in
ComputedStyleConstants.h with the restricted set of allowed values and a mapping
is added to CSSPrimitiveValueMappings.h.
You should do this here to maintain consistency with the rest of ComputedStyle
if the number of possible values is more than two.

[nainar, 2016-03-21 00:12:08]

Hi!

PTAL?

Thanks!

https://codereview.chromium.org/1812763002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/style/ContentData.h (right):

https://codereview.chromium.org/1812763002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/style/ContentData.h:229: CSSValueID m_cssValueID;
Done.

https://codereview.chromium.org/1812763002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp
(right):

https://codereview.chromium.org/1812763002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp:978:
CSSValueID valueForElementContent(const ElementContent elementContent)
Added the mapping here as QuoteType the other enum used in ContentData is also
mapped here.

https://codereview.chromium.org/1812763002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/style/ComputedStyleConstants.h (right):

https://codereview.chromium.org/1812763002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/style/ComputedStyleConstants.h:310: enum
ElementContent {
Better(?) name: ElementContentValue

Also added an enum even though it can have only two values, since there is
another spec under draft (has a do not implement as of yet message) which
introduces a new value: inhibit. Also a lot of other enums have only two values.

https://codereview.chromium.org/1812763002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/style/ContentData.h (right):

https://codereview.chromium.org/1812763002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/style/ContentData.h:205: class
ElementContentContentData final : public ContentData {
Better(?) Name: ElementContentValueContentData

[nainar, 2016-03-22 00:45:33]

Sorry, seemed to have missed a failing tests :/

PTAL? Thanks!

[alancutter (OOO until 2018), 2016-03-22 02:25:03]

Summary of in person discussions:
 - We want to be targeting the CSS2 spec as CSS3 is not yet stable:
https://drafts.csswg.org/css2/generate.html#content
 - Need to add support for "none" to the CSSPropertyParser.
 - We should parse "none" and "normal" as single idents rather than as list
items.
 - The StyleBuilder should always clear content for elements.
 - The StyleBuilder should always clear content if "normal" is set on pseudo
elements.
 - getComputedStyle() can revert back to your initial patch.

[nainar, 2016-03-22 03:32:16]

Description was changed from

==========
Pseudo and non pseudo elements should return correct computed content value

This patch makes sure that content computes to "normal" for elements
(not pseudo elements) and to "none" if the specified value of pseudo
elements is "normal."

This is interoperable with IE, Edge returns an empty string for non
pseudo elements while FF returns none.

The spec is given here: 
https://drafts.csswg.org/css2/generate.html#content

BUG=313670
==========

to

==========
Pseudo and non pseudo elements should return correct computed content value

This patch makes sure that content computes to "normal" for elements
(not pseudo elements) and to "none" if the specified value of pseudo
elements is "normal." This patch also adds support for "none" to the 
CSSPropertyParser and ensures that we parse "none" and "normal" as 
single idents rather than as list items.

This is interoperable with IE, Edge returns an empty string for non
pseudo elements while FF returns none.

The spec implemented is given here: 
https://drafts.csswg.org/css2/generate.html#content
==========

[nainar, 2016-03-22 03:41:30]

@alancutter,

PTAL?

Thanks!

[nainar, 2016-03-22 05:48:29]

Description was changed from

==========
Pseudo and non pseudo elements should return correct computed content value

This patch makes sure that content computes to "normal" for elements
(not pseudo elements) and to "none" if the specified value of pseudo
elements is "normal." This patch also adds support for "none" to the 
CSSPropertyParser and ensures that we parse "none" and "normal" as 
single idents rather than as list items.

This is interoperable with IE, Edge returns an empty string for non
pseudo elements while FF returns none.

The spec implemented is given here: 
https://drafts.csswg.org/css2/generate.html#content
==========

to

==========
Pseudo and non pseudo elements should return correct computed content value

This patch makes sure that content computes to "normal" for elements
(not pseudo elements) and to "none" if the specified value of pseudo
elements is "normal." This patch also adds support for "none" to the 
CSSPropertyParser and ensures that we parse "none" and "normal" as 
single idents rather than as list items.

This is interoperable with IE, Edge returns an empty string for non
pseudo elements while FF returns none.

The spec implemented is given here: 
https://drafts.csswg.org/css2/generate.html#content

BUG=313670
==========

[alancutter (OOO until 2018), 2016-03-22 09:35:24]

https://codereview.chromium.org/1812763002/diff/80001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/dom/hover-after-dom-delete.html
(right):

https://codereview.chromium.org/1812763002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/dom/hover-after-dom-delete.html:103:
function checkBlueHoverText(expectedText)
This should probably be renamed to checkBlueHoverContent since it's not checking
the actual text.

https://codereview.chromium.org/1812763002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp
(right):

https://codereview.chromium.org/1812763002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp:2385: return
cssValuePool().createIdentifierValue(CSSValueNone);
I think you should be using styledNode->isPseudoElement() and friends.

https://codereview.chromium.org/1812763002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/1812763002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:2342:
parsedValue = consumeIdent<CSSValueOpenQuote, CSSValueCloseQuote,
CSSValueNoOpenQuote, CSSValueNoCloseQuote, CSSValueNormal>(range);
We shouldn't be accepting CSSValueNormal here.

https://codereview.chromium.org/1812763002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp (right):

https://codereview.chromium.org/1812763002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp:660: if
(!value->isValueList()) {
Assert that it is normal or none.

[Timothy Loh, 2016-03-22 23:25:27]

https://codereview.chromium.org/1812763002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/1812763002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:2333:
RefPtrWillBeRawPtr<CSSValue> parsedValue = consumeIdent<CSSValueNone,
CSSValueNormal>(range);
btw we've been using identMatches elsewhere, that's probably nicer instead of
scope-creeping parsedValue.

[nainar, 2016-03-23 02:50:49]

Hi, 

PTAL? 

Thanks!

https://codereview.chromium.org/1812763002/diff/80001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/dom/hover-after-dom-delete.html
(right):

https://codereview.chromium.org/1812763002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/dom/hover-after-dom-delete.html:103:
function checkBlueHoverText(expectedText)
Done.

https://codereview.chromium.org/1812763002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp
(right):

https://codereview.chromium.org/1812763002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp:2385: return
cssValuePool().createIdentifierValue(CSSValueNone);
Done.

https://codereview.chromium.org/1812763002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right):

https://codereview.chromium.org/1812763002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:2333:
RefPtrWillBeRawPtr<CSSValue> parsedValue = consumeIdent<CSSValueNone,
CSSValueNormal>(range);
Using identMatches now

https://codereview.chromium.org/1812763002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:2342:
parsedValue = consumeIdent<CSSValueOpenQuote, CSSValueCloseQuote,
CSSValueNoOpenQuote, CSSValueNoCloseQuote, CSSValueNormal>(range);
Done.

https://codereview.chromium.org/1812763002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp (right):

https://codereview.chromium.org/1812763002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp:660: if
(!value->isValueList()) {
Done.

[alancutter (OOO until 2018), 2016-03-23 03:12:30]

lgtm

https://codereview.chromium.org/1812763002/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/css/getComputedStyle/disallowed-url-serialization.html
(right):

https://codereview.chromium.org/1812763002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/css/getComputedStyle/disallowed-url-serialization.html:11:
['content', 'url("chrome:")', 'normal'],
Revert changes and just remove this test case since it no longer applies.

[nainar, 2016-03-23 03:19:18]

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

[nainar, 2016-03-23 03:19:19]

@timloh

PTAL?

Thanks!

https://codereview.chromium.org/1812763002/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/css/getComputedStyle/disallowed-url-serialization.html
(right):

https://codereview.chromium.org/1812763002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/css/getComputedStyle/disallowed-url-serialization.html:11:
['content', 'url("chrome:")', 'normal'],
Done.

[nainar, 2016-03-23 04:42:10]

Description was changed from

==========
Pseudo and non pseudo elements should return correct computed content value

This patch makes sure that content computes to "normal" for elements
(not pseudo elements) and to "none" if the specified value of pseudo
elements is "normal." This patch also adds support for "none" to the 
CSSPropertyParser and ensures that we parse "none" and "normal" as 
single idents rather than as list items.

This is interoperable with IE, Edge returns an empty string for non
pseudo elements while FF returns none.

The spec implemented is given here: 
https://drafts.csswg.org/css2/generate.html#content

BUG=313670
==========

to

==========
Pseudo and non pseudo elements should return correct computed content value

This patch makes sure that content computes to "normal" for elements
(not pseudo elements) and to "none" if the specified value of pseudo
elements is "normal." This patch also adds support for "none" to the 
CSSPropertyParser and ensures that we parse "none" and "normal" as 
single idents rather than as list items.

This patch makes us interoperable with all other browsers for the
case of pseudo elements where the specified value is "normal".
With regards to non psuedo elements we are now interoperable with IE,
while Edge returns an empty string for non pseudo elements 
and FF returns none.

The spec implemented is given here: 
https://drafts.csswg.org/css2/generate.html#content

BUG=313670
==========

[Timothy Loh, 2016-03-23 05:07:49]

I think the description needs to be clearer. Try to explain *exactly* what the
behavior changes are and what other browsers do in these cases. For example, the
code change is doing something about :before/:after, but this isn't mentioned in
the description.

Also it looks like we're now rejecting 'content: normal normal;', this needs to
be mentioned in the patch description and also tested (in fact it would be
better for this to be a separate patch).

https://codereview.chromium.org/1812763002/diff/120001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/css3/supports.html (right):

https://codereview.chromium.org/1812763002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/css3/supports.html:254:
shouldBeEqualToString("getComputedStyle(document.getElementById('t"+i+"')).content",
"normal");
You're pretty much making this test useless. You should work out the purpose of
tests which you touch so you can make appropriate changes, instead of just
rebaselining to try and make them pass. Please have another look over the other
tests you've changed too.

https://codereview.chromium.org/1812763002/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/css/getComputedStyle/disallowed-url-serialization.html
(left):

https://codereview.chromium.org/1812763002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/css/getComputedStyle/disallowed-url-serialization.html:11:
['content', 'url("chrome:")'],
Why delete this test case? It'd be better to update the test so this is still
covered.

[nainar, 2016-03-23 05:19:34]

Description was changed from

==========
Pseudo and non pseudo elements should return correct computed content value

This patch makes sure that content computes to "normal" for elements
(not pseudo elements) and to "none" if the specified value of pseudo
elements is "normal." This patch also adds support for "none" to the 
CSSPropertyParser and ensures that we parse "none" and "normal" as 
single idents rather than as list items.

This patch makes us interoperable with all other browsers for the
case of pseudo elements where the specified value is "normal".
With regards to non psuedo elements we are now interoperable with IE,
while Edge returns an empty string for non pseudo elements 
and FF returns none.

The spec implemented is given here: 
https://drafts.csswg.org/css2/generate.html#content

BUG=313670
==========

to

==========
Pseudo and non pseudo elements should return correct computed content value

This patch makes sure that content computes to "normal" for elements
(not pseudo elements) and to "none" if the specified value of pseudo
elements ::before and ::after is "normal."

This patch makes us interoperable with all other browsers for the
case of pseudo elements :before and ::after where the specified value 
is "normal".

With regards to non psuedo elements we are now interoperable with IE 
and the spec and return "normal" for the computed content all all non 
psuedo elements.
Other browsers are inconcistent on this behaviour:
1. Edge returns an empty string for non pseudo elements
2. FF returns none for non pseudo elements.

The spec implemented is given here: 
https://drafts.csswg.org/css2/generate.html#content

BUG=313670
==========

[alancutter (OOO until 2018), 2016-03-23 05:30:30]

https://codereview.chromium.org/1812763002/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/css/getComputedStyle/disallowed-url-serialization.html
(left):

https://codereview.chromium.org/1812763002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/css/getComputedStyle/disallowed-url-serialization.html:11:
['content', 'url("chrome:")'],
On 2016/03/23 at 05:07:49, Timothy Loh wrote:
> Why delete this test case? It'd be better to update the test so this is still
covered.

I wrote this test and consider this reasonable to remove. The code paths of
interest for this test are still being tested, many of these properties are
redundant with respect to our implementation.

[nainar, 2016-03-23 06:50:41]

@timloh,
 
Changed the description as well.
PTAL?

Thanks!

https://codereview.chromium.org/1812763002/diff/120001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/css3/supports.html (right):

https://codereview.chromium.org/1812763002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/css3/supports.html:254:
shouldBeEqualToString("getComputedStyle(document.getElementById('t"+i+"')).content",
"normal");
Fixed. In this and nested-at-rules.html

https://codereview.chromium.org/1812763002/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/css/getComputedStyle/disallowed-url-serialization.html
(left):

https://codereview.chromium.org/1812763002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/css/getComputedStyle/disallowed-url-serialization.html:11:
['content', 'url("chrome:")'],
Retaining changes as per above.

[nainar, 2016-03-24 04:57:33]

Description was changed from

==========
Pseudo and non pseudo elements should return correct computed content value

This patch makes sure that content computes to "normal" for elements
(not pseudo elements) and to "none" if the specified value of pseudo
elements ::before and ::after is "normal."

This patch makes us interoperable with all other browsers for the
case of pseudo elements :before and ::after where the specified value 
is "normal".

With regards to non psuedo elements we are now interoperable with IE 
and the spec and return "normal" for the computed content all all non 
psuedo elements.
Other browsers are inconcistent on this behaviour:
1. Edge returns an empty string for non pseudo elements
2. FF returns none for non pseudo elements.

The spec implemented is given here: 
https://drafts.csswg.org/css2/generate.html#content

BUG=313670
==========

to

==========
Pseudo and non pseudo elements should return correct computed content value

This patch makes sure that content computes to "normal" for elements
(not pseudo elements) and to "none" if the specified value of pseudo
elements ::before and ::after is "normal."

This patch makes us interoperable with all other browsers for the
case of pseudo elements :before and ::after where the specified value 
is "normal".

With regards to non psuedo elements we are now interoperable with IE 
and the spec and return "normal" for the computed content all all non 
psuedo elements.
Other browsers are inconcistent on this behaviour:
1. Edge returns an empty string for non pseudo elements
2. FF returns none for non pseudo elements.

The spec implemented is given here: 
https://drafts.csswg.org/css2/generate.html#content

BUG=597500
==========

[nainar, 2016-03-29 08:20:26]

@timloh,

Pinging to mention that I have updated the patch since I last sent it. Merge
issues with that one. PTAL? 

Thanks!

[Timothy Loh, 2016-03-30 05:59:42]

On 2016/03/29 08:20:26, nainar wrote:
> @timloh,
> 
> Pinging to mention that I have updated the patch since I last sent it. Merge
> issues with that one. PTAL? 
> 
> Thanks!

lgtm

[nainar, 2016-03-30 06:02:03]

The CQ bit was checked by nainar@chromium.org

[nainar, 2016-03-30 06:02:03]

The patchset sent to the CQ was uploaded after l-g-t-m from
alancutter@chromium.org
Link to the patchset: https://codereview.chromium.org/1812763002/#ps260001
(title: " ")

[commit-bot: I haz the power, 2016-03-30 06:02:25]

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

[commit-bot: I haz the power, 2016-03-30 06:02:26]

This CL has an open dependency (Issue 1837413002 Patch 60001). Please resolve
the dependency and try again.

[nainar, 2016-03-30 11:42:30]

The CQ bit was checked by nainar@chromium.org

[commit-bot: I haz the power, 2016-03-30 11:42:38]

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

[commit-bot: I haz the power, 2016-03-30 12:16:01]

Committed patchset #14 (id:260001)

[commit-bot: I haz the power, 2016-03-30 12:17:33]

Description was changed from

==========
Pseudo and non pseudo elements should return correct computed content value

This patch makes sure that content computes to "normal" for elements
(not pseudo elements) and to "none" if the specified value of pseudo
elements ::before and ::after is "normal."

This patch makes us interoperable with all other browsers for the
case of pseudo elements :before and ::after where the specified value 
is "normal".

With regards to non psuedo elements we are now interoperable with IE 
and the spec and return "normal" for the computed content all all non 
psuedo elements.
Other browsers are inconcistent on this behaviour:
1. Edge returns an empty string for non pseudo elements
2. FF returns none for non pseudo elements.

The spec implemented is given here: 
https://drafts.csswg.org/css2/generate.html#content

BUG=597500
==========

to

==========
Pseudo and non pseudo elements should return correct computed content value

This patch makes sure that content computes to "normal" for elements
(not pseudo elements) and to "none" if the specified value of pseudo
elements ::before and ::after is "normal."

This patch makes us interoperable with all other browsers for the
case of pseudo elements :before and ::after where the specified value
is "normal".

With regards to non psuedo elements we are now interoperable with IE
and the spec and return "normal" for the computed content all all non
psuedo elements.
Other browsers are inconcistent on this behaviour:
1. Edge returns an empty string for non pseudo elements
2. FF returns none for non pseudo elements.

The spec implemented is given here:
https://drafts.csswg.org/css2/generate.html#content

BUG=597500

Committed: https://crrev.com/aaeefae6c3a945e7fcfef382c3a82002ffae0e19
Cr-Commit-Position: refs/heads/master@{#383949}
==========

[commit-bot: I haz the power, 2016-03-30 12:17:34]

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

[nainar, 2016-05-16 05:31:54]

A revert of this CL (patchset #14 id:260001) has been created in
https://codereview.chromium.org/1977323002/ by nainar@chromium.org.

The reason for reverting is: BUG=609848

A lot of sites in production are depending on content returning an empty
string..