text-decoration:none no longer valid

[Ojan Vafai]

http://plexode.com/eval3/#jt=document.body.style.textDecoration%20%3D%20'none'%3B%0Aalert(document.body.style.textDecoration)

That alerts 'none' in Safari 6 and Chrome 22. It alerts 'initial' at tip of tree.

See also http://crbug.com/159938.

[Bruno Abinader (history only)]

(In reply to comment #0)
> http://plexode.com/eval3/#jt=document.body.style.textDecoration%20%3D%20'none'%3B%0Aalert(document.body.style.textDecoration)
> 
> That alerts 'none' in Safari 6 and Chrome 22. It alerts 'initial' at tip of tree.
> 
> See also http://crbug.com/159938.

I confirm the issue and also tested on Firefox 16 and Opera 12 with 'none' as result of the JS evaluation. Said this, assigning this to myself.

[Bruno Abinader (history only)]

From a first look this doesn't seem like a regression issue. The "text-decoration" property follows CSS2.1 implementation, which remains unaffected by the recent addition of CSS3 text decoration properties, such as "text-decoration-line" (see bug 90509 for details). In fact, the computed style of webkitTextDecoration remains "none", while textDecoration gets "initial" when applying "none" to both (see link below):

http://plexode.com/eval3/#jt=document.body.style.textDecoration%20%3D%20'none'%3B%0Adocument.body.style.webkitTextDecoration%20%3D%20'none'%3B%0Astring%20%3D%20%22textDecoration%3A%20%22%20%2B%20document.body.style.textDecoration%20%2B%20%22%5CnwebkitTextDecoration%3A%20%22%20%2B%20document.body.style.webkitTextDecoration%3B

Thanks to Julien, the getComputedStyle tests on "-webkit-text-decoration-line" got quite robust and this specific test is handled in fast/css3-text/css3-text-decoration/getComputedStyle/getComputedStyle-text-decoration-line.html.

Said this, I am going to fix this for "text-decoration" and update the fast/css/getComputedStyle/getComputedStyle-text-decoration.html layout test with all the possible combinations, to be sure we catch these kind of issues while running layout tests.

[Bruno Abinader (history only)]

While investigating this issue, I've found out that 'blink' value was not properly accepted by "text-decoration" property, after the changes I've made to the function parser. However, this does not have correlation with this issue. The updated layout test for 'text-decoration' shows the following result:

Value 'none':
PASS e.style.textDecoration is 'initial'
PASS e.style.getPropertyCSSValue('text-decoration').toString() is '[object CSSValue]'
PASS e.style.getPropertyCSSValue('text-decoration').cssText is 'initial'
PASS computedStyle.textDecoration is 'none'
PASS computedStyle.getPropertyCSSValue('text-decoration').toString() is '[object CSSPrimitiveValue]'
PASS computedStyle.getPropertyCSSValue('text-decoration').cssText is 'none'

This indicates that the property value right after parsing gets the 'initial' value, however the computed style is correct - "none". Is it possible that the JS evaluator is not getting the computed style?

[Bruno Abinader (history only)]

I believe I found the reason for this issue to happen:

diff --git a/Source/WebCore/css/CSSParser.cpp b/Source/WebCore/css/CSSParser.cpp
index bb09d30..7ab7387 100644
--- a/Source/WebCore/css/CSSParser.cpp
+++ b/Source/WebCore/css/CSSParser.cpp
@@ -8236,7 +8236,7 @@ bool CSSParser::parseTextDecoration(CSSPropertyID propId, bool important)
 {
     CSSParserValue* value = m_valueList->current();
     if (value->id == CSSValueNone) {
-        addTextDecorationProperty(propId, cssValuePool().createExplicitInitialValue(), important);
+        addTextDecorationProperty(propId, cssValuePool().createIdentifierValue(CSSValueNone), important);
         m_valueList->next();
         return true;
     }

With this change applied, the updated layout test for "text-decoration" added in bug 101750 gets the following result:

Value 'none':
FAIL e.style.textDecoration should be initial. Was none.
FAIL e.style.getPropertyCSSValue('text-decoration').toString() should be [object CSSValue]. Was [object CSSPrimitiveValue].
FAIL e.style.getPropertyCSSValue('text-decoration').cssText should be initial. Was none.
PASS computedStyle.textDecoration is 'none'
PASS computedStyle.getPropertyCSSValue('text-decoration').toString() is '[object CSSPrimitiveValue]'
PASS computedStyle.getPropertyCSSValue('text-decoration').cssText is 'none'

Said this, I am going to provide a patch for this issue as soon as patch from bug 101750 gets landed.

[Bruno Abinader (history only)]

Created attachment 173327 [details]
Patch

Proposed patch

[WebKit Review Bot]

Comment on attachment 173327 [details]
Patch

Rejecting attachment 173327 [details] from commit-queue.

New failing tests:
inspector/console/console-format-style-whitelist.html
Full output: http://queues.webkit.org/results/14771824

[WebKit Review Bot]

Comment on attachment 173327 [details]
Patch

Attachment 173327 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14775748

New failing tests:
inspector/console/console-format-style-whitelist.html

[Build Bot]

Comment on attachment 173327 [details]
Patch

Attachment 173327 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14781545

New failing tests:
inspector/console/console-format-style-whitelist.html

[Bruno Abinader (history only)]

Created attachment 173448 [details]
Patch

Fixed test result for 'inspector/console/console-format-style-whitelist.html' layout test (s/initial/none).

[WebKit Review Bot]

Comment on attachment 173448 [details]
Patch

Clearing flags on attachment: 173448

Committed r134156: <http://trac.webkit.org/changeset/134156>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.