Bug 127361 – ASSERTION FAILED: std::isfinite(num) in WebCore::CSSPrimitiveValue::CSSPrimitiveValue

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Renata Hodovan]

Created attachment 221771 [details]
Test case

The failing test case:

<svg xmlns="http://www.w3.org/2000/svg">
	<polyline font-size="8E+617%">
</svg>


The backtrace:

ASSERTION FAILED: std::isfinite(num)
/home/reni2/data/REPOS/webkit/Source/WebCore/css/CSSPrimitiveValue.cpp(259) : WebCore::CSSPrimitiveValue::CSSPrimitiveValue(double, WebCore::CSSPrimitiveValue::UnitTypes)
1   0x7ffff5c17fd1 WTFCrash
2   0x7ffff0e77e6d WebCore::CSSPrimitiveValue::CSSPrimitiveValue(double, WebCore::CSSPrimitiveValue::UnitTypes)
3   0x7ffff0dcac91 WebCore::CSSPrimitiveValue::create(double, WebCore::CSSPrimitiveValue::UnitTypes)
4   0x7ffff0e9c6f2 WebCore::CSSValuePool::createValue(double, WebCore::CSSPrimitiveValue::UnitTypes)
5   0x7ffff0e323ed
6   0x7ffff0e33c63 WebCore::CSSParser::parseValue(WebCore::MutableStyleProperties*, WebCore::CSSPropertyID, WTF::String const&, bool, WebCore::CSSParserMode, WebCore::StyleSheetContents*)
7   0x7ffff0f1ea01 WebCore::MutableStyleProperties::setProperty(WebCore::CSSPropertyID, WTF::String const&, bool, WebCore::StyleSheetContents*)
8   0x7ffff1070f6b WebCore::StyledElement::addPropertyToPresentationAttributeStyle(WebCore::MutableStyleProperties&, WebCore::CSSPropertyID, WTF::String const&)
9   0x7ffff1acf852 WebCore::SVGElement::collectStyleForPresentationAttribute(WebCore::QualifiedName const&, WTF::AtomicString const&, WebCore::MutableStyleProperties&)
10  0x7ffff1070b21 WebCore::StyledElement::rebuildPresentationAttributeStyle()
11  0x7ffff0ee8003 WebCore::StyledElement::presentationAttributeStyle()
12  0x7ffff0ee6dfc WebCore::ElementRuleCollector::matchAllRules(bool, bool)
13  0x7ffff0f2812c WebCore::StyleResolver::styleForElement(WebCore::Element*, WebCore::RenderStyle*, WebCore::StyleSharingBehavior, WebCore::RuleMatchingBehavior, WebCore::RenderRegion*)
14  0x7ffff1ace2a6 WebCore::SVGElement::customStyleForRenderer()
15  0x7ffff0fe9fab WebCore::Element::styleForRenderer()
16  0x7ffff1a75994
17  0x7ffff1a76c00
18  0x7ffff1a767b8
19  0x7ffff1a76cd9
20  0x7ffff1a767b8
21  0x7ffff1a76cd9
22  0x7ffff1a767b8
23  0x7ffff1a76cd9
24  0x7ffff1a77399
25  0x7ffff1a7793b
26  0x7ffff1a77dbd WebCore::Style::resolveTree(WebCore::Document&, WebCore::Style::Change)
27  0x7ffff0f93baa WebCore::Document::recalcStyle(WebCore::Style::Change)
28  0x7ffff0f93e67 WebCore::Document::updateStyleIfNeeded()
29  0x7ffff0f9d2bb WebCore::Document::finishedParsing()
30  0x7ffff128cdd9 WebCore::HTMLConstructionSite::finishedParsing()
31  0x7ffff12c5def WebCore::HTMLTreeBuilder::finished()

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff5c17fd6 in WTFCrash () at /home/reni2/data/REPOS/webkit/Source/WTF/wtf/Assertions.cpp:333
333	    *(int *)(uintptr_t)0xbbadbeef = 0;
(gdb) bt
#0  0x00007ffff5c17fd6 in WTFCrash () at /home/reni2/data/REPOS/webkit/Source/WTF/wtf/Assertions.cpp:333
#1  0x00007ffff0e77e6d in WebCore::CSSPrimitiveValue::CSSPrimitiveValue (this=0x75ee40, num=inf, type=WebCore::CSSPrimitiveValue::CSS_PERCENTAGE)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/css/CSSPrimitiveValue.cpp:259
#2  0x00007ffff0dcac91 in WebCore::CSSPrimitiveValue::create (value=inf, type=WebCore::CSSPrimitiveValue::CSS_PERCENTAGE)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/css/CSSPrimitiveValue.h:214
#3  0x00007ffff0e9c6f2 in WebCore::CSSValuePool::createValue (this=0x767fb0, value=inf, type=WebCore::CSSPrimitiveValue::CSS_PERCENTAGE)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/css/CSSValuePool.cpp:93
#4  0x00007ffff0e323ed in WebCore::parseSimpleLengthValue (declaration=0xfc23e0, propertyId=WebCore::CSSPropertyFontSize, string=..., 
    important=false, cssParserMode=WebCore::SVGAttributeMode) at /home/reni2/data/REPOS/webkit/Source/WebCore/css/CSSParser.cpp:644
#5  0x00007ffff0e33c63 in WebCore::CSSParser::parseValue (declaration=0xfc23e0, propertyID=WebCore::CSSPropertyFontSize, string=..., important=false, 
    cssParserMode=WebCore::SVGAttributeMode, contextStyleSheet=0xfc2450) at /home/reni2/data/REPOS/webkit/Source/WebCore/css/CSSParser.cpp:1298
#6  0x00007ffff0f1ea01 in WebCore::MutableStyleProperties::setProperty (this=0xfc23e0, propertyID=WebCore::CSSPropertyFontSize, value=..., 
    important=false, contextStyleSheet=0xfc2450) at /home/reni2/data/REPOS/webkit/Source/WebCore/css/StyleProperties.cpp:676
#7  0x00007ffff1070f6b in WebCore::StyledElement::addPropertyToPresentationAttributeStyle (this=0x9ca070, style=..., 
    propertyID=WebCore::CSSPropertyFontSize, value=...) at /home/reni2/data/REPOS/webkit/Source/WebCore/dom/StyledElement.cpp:379
#8  0x00007ffff1acf852 in WebCore::SVGElement::collectStyleForPresentationAttribute (this=0x9ca070, name=..., value=..., style=...)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/svg/SVGElement.cpp:990
#9  0x00007ffff1070b21 in WebCore::StyledElement::rebuildPresentationAttributeStyle (this=0x9ca070)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/dom/StyledElement.cpp:341
#10 0x00007ffff0ee8003 in WebCore::StyledElement::presentationAttributeStyle (this=0x9ca070)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/dom/StyledElement.h:104
#11 0x00007ffff0ee6dfc in WebCore::ElementRuleCollector::matchAllRules (this=0x7fffffffb410, matchAuthorAndUserStyles=true, 
    includeSMILProperties=true) at /home/reni2/data/REPOS/webkit/Source/WebCore/css/ElementRuleCollector.cpp:434
#12 0x00007ffff0f2812c in WebCore::StyleResolver::styleForElement (this=0x9cb540, element=0x9ca070, defaultParent=0x0, 
    sharingBehavior=WebCore::AllowStyleSharing, matchingBehavior=WebCore::MatchAllRules, regionForStyling=0x0)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/css/StyleResolver.cpp:874
#13 0x00007ffff1ace2a6 in WebCore::SVGElement::customStyleForRenderer (this=0x9ca070)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/svg/SVGElement.cpp:768
#14 0x00007ffff0fe9fab in WebCore::Element::styleForRenderer (this=0x9ca070) at /home/reni2/data/REPOS/webkit/Source/WebCore/dom/Element.cpp:1453
#15 0x00007ffff1a75994 in WebCore::Style::createRendererIfNeeded (element=..., resolvedStyle=...)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/style/StyleResolveTree.cpp:221
#16 0x00007ffff1a76c00 in WebCore::Style::attachRenderTree (current=..., resolvedStyle=...)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/style/StyleResolveTree.cpp:544
#17 0x00007ffff1a767b8 in WebCore::Style::attachChildren (current=...) at /home/reni2/data/REPOS/webkit/Source/WebCore/style/StyleResolveTree.cpp:469
#18 0x00007ffff1a76cd9 in WebCore::Style::attachRenderTree (current=..., resolvedStyle=...)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/style/StyleResolveTree.cpp:560
#19 0x00007ffff1a767b8 in WebCore::Style::attachChildren (current=...) at /home/reni2/data/REPOS/webkit/Source/WebCore/style/StyleResolveTree.cpp:469
#20 0x00007ffff1a76cd9 in WebCore::Style::attachRenderTree (current=..., resolvedStyle=...)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/style/StyleResolveTree.cpp:560
#21 0x00007ffff1a767b8 in WebCore::Style::attachChildren (current=...) at /home/reni2/data/REPOS/webkit/Source/WebCore/style/StyleResolveTree.cpp:469
#22 0x00007ffff1a76cd9 in WebCore::Style::attachRenderTree (current=..., resolvedStyle=...)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/style/StyleResolveTree.cpp:560
#23 0x00007ffff1a77399 in WebCore::Style::resolveLocal (current=..., inheritedChange=WebCore::Style::NoChange)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/style/StyleResolveTree.cpp:684
#24 0x00007ffff1a7793b in WebCore::Style::resolveTree (current=..., change=WebCore::Style::NoChange)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/style/StyleResolveTree.cpp:838
#25 0x00007ffff1a77dbd in WebCore::Style::resolveTree (document=..., change=WebCore::Style::NoChange)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/style/StyleResolveTree.cpp:912
#26 0x00007ffff0f93baa in WebCore::Document::recalcStyle (this=0x725e10, change=WebCore::Style::NoChange)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/dom/Document.cpp:1752
#27 0x00007ffff0f93e67 in WebCore::Document::updateStyleIfNeeded (this=0x725e10) at /home/reni2/data/REPOS/webkit/Source/WebCore/dom/Document.cpp:1804
#28 0x00007ffff0f9d2bb in WebCore::Document::finishedParsing (this=0x725e10) at /home/reni2/data/REPOS/webkit/Source/WebCore/dom/Document.cpp:4452
#29 0x00007ffff128cdd9 in WebCore::HTMLConstructionSite::finishedParsing (this=0x792f28)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/html/parser/HTMLConstructionSite.cpp:337
#30 0x00007ffff12c5def in WebCore::HTMLTreeBuilder::finished (this=0x792f10)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/html/parser/HTMLTreeBuilder.cpp:3046
---Type <return> to continue, or q <return> to quit---
#31 0x00007ffff1294074 in WebCore::HTMLDocumentParser::end (this=0x9328b0)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/html/parser/HTMLDocumentParser.cpp:439
#32 0x00007ffff129415f in WebCore::HTMLDocumentParser::attemptToRunDeferredScriptsAndEnd (this=0x9328b0)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/html/parser/HTMLDocumentParser.cpp:450
#33 0x00007ffff1292da9 in WebCore::HTMLDocumentParser::prepareToStopParsing (this=0x9328b0)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/html/parser/HTMLDocumentParser.cpp:165
#34 0x00007ffff12941a2 in WebCore::HTMLDocumentParser::attemptToEnd (this=0x9328b0)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/html/parser/HTMLDocumentParser.cpp:462
#35 0x00007ffff1294259 in WebCore::HTMLDocumentParser::finish (this=0x9328b0)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/html/parser/HTMLDocumentParser.cpp:490
#36 0x00007ffff14082fd in WebCore::DocumentWriter::end (this=0x77e5d0) at /home/reni2/data/REPOS/webkit/Source/WebCore/loader/DocumentWriter.cpp:248
#37 0x00007ffff13f311b in WebCore::DocumentLoader::finishedLoading (this=0x77e530, finishTime=0)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/loader/DocumentLoader.cpp:440
#38 0x00007ffff13f2e84 in WebCore::DocumentLoader::notifyFinished (this=0x77e530, resource=0x9ba6a0)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/loader/DocumentLoader.cpp:374
#39 0x00007ffff149822e in WebCore::CachedResource::checkNotify (this=0x9ba6a0)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/loader/cache/CachedResource.cpp:336
#40 0x00007ffff149830c in WebCore::CachedResource::finishLoading (this=0x9ba6a0)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/loader/cache/CachedResource.cpp:352
#41 0x00007ffff1494d76 in WebCore::CachedRawResource::finishLoading (this=0x9ba6a0, data=0x823a50)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/loader/cache/CachedRawResource.cpp:94
#42 0x00007ffff1451a5e in WebCore::SubresourceLoader::didFinishLoading (this=0x9babd0, finishTime=0)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/loader/SubresourceLoader.cpp:309
#43 0x00007ffff144dd83 in WebCore::ResourceLoader::didFinishLoading (this=0x9babd0, finishTime=0)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/loader/ResourceLoader.cpp:517
#44 0x00007ffff21a3f18 in WebCore::readCallback (asyncResult=0x9301c0, data=0x8344e0)
    at /home/reni2/data/REPOS/webkit/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:1336
#45 0x00007fffe7f9701a in async_ready_callback_wrapper (source_object=0x91d9e0, res=0x9301c0, user_data=0x8344e0) at ginputstream.c:530
#46 0x00007fffe7fb65ab in g_task_return_now (task=0x9301c0) at gtask.c:1105
#47 0x00007fffe7fb65c9 in complete_in_idle_cb (task=0x9301c0) at gtask.c:1114
#48 0x00007fffed77ef46 in g_main_dispatch (context=0x92f9a0) at gmain.c:3054
#49 g_main_context_dispatch (context=context@entry=0x92f9a0) at gmain.c:3630
#50 0x00007ffff75676e8 in _ecore_glib_select__locked (ecore_timeout=<optimized out>, efds=<optimized out>, wfds=0x7fffffffc690, rfds=0x7fffffffc610, 
    ecore_fds=10, ctx=<optimized out>) at ecore_glib.c:171
#51 _ecore_glib_select (ecore_fds=10, rfds=0x7fffffffc610, wfds=0x7fffffffc690, efds=<optimized out>, ecore_timeout=<optimized out>)
    at ecore_glib.c:205
#52 0x00007ffff7561b37 in _ecore_main_select (timeout=timeout@entry=0) at ecore_main.c:1466
#53 0x00007ffff756262c in _ecore_main_loop_iterate_internal (once_only=once_only@entry=0) at ecore_main.c:1860
#54 0x00007ffff75629c7 in ecore_main_loop_begin () at ecore_main.c:956
#55 0x0000000000406c6b in main (argc=2, argv=0x7fffffffdb28) at /home/reni2/data/REPOS/webkit/Tools/EWebLauncher/main.c:1026

[Martin Hodovan]

Created attachment 221772 [details]
Proposed patch

I am not completely sure that the value check is at the right place, so if anyone has a better idea please let me know.

[Darin Adler]

Comment on attachment 221772 [details]
Proposed patch

I’m not sure this is the right fix and a helpful change. Turning the colossally large number into 0 just to avoid the assertion doesn’t necessarily help improve the behavior of the engine, other than sidestepping an assertion that may not be correct. It might be more appropriate to turn such values into parse failures. Not sure it matters either way. Loosening the assertion also might be sufficient. I especially regret adding more parsing special cases here since it might prevent us from doing clean-up refactoring in the future and even has a small performance cost.

[Martin Hodovan]

Created attachment 223339 [details]
Proposed patch

[Martin Hodovan]

The previous patch got updated according to Darin's review. (parsing failure)

[Zoltan Horvath]

(In reply to comment #4)
> The previous patch got updated according to Darin's review. (parsing failure)

The patch you uploaded contains 2 changelog modifications. Please pay attention to upload the diff to the original repository.

[Martin Hodovan]

Created attachment 223372 [details]
Proposed patch

Sorry about this mistake, I attached the patch again.

[Build Bot]

Comment on attachment 223372 [details]
Proposed patch

Attachment 223372 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4653103882174464

New failing tests:
fast/css/parsing-infinite-floating-value.html

[Build Bot]

Created attachment 223375 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 223372 [details]
Proposed patch

Attachment 223372 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5610099905134592

New failing tests:
fast/css/parsing-infinite-floating-value.html

[Build Bot]

Created attachment 223381 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 223372 [details]
Proposed patch

Attachment 223372 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6622795748343808

New failing tests:
fast/css/parsing-infinite-floating-value.html

[Build Bot]

Created attachment 223392 [details]
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-02  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Benjamin Poulain]

Comment on attachment 223372 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=223372&action=review

Looks good but. The test fails.

> LayoutTests/fast/css/parsing-infinite-floating-value.html:16
> +	<head>
> +		<script>
> +				if (window.testRunner)
> +				    testRunner.dumpAsText();
> +		</script>
> +	</head>
> +
> +	<body>
> +		<p>This test passes if it does not crash.</p>
> +
> +		<svg xmlns="http://www.w3.org/2000/svg" >
> +			<polyline font-size="8E+617%"></polyline>
> +		</svg>
> +
> +	</body>

Let's fix the indent here.

Can you please extend the test to non SVG values? It would be useful to test some HTML.

[Martin Hodovan]

I have run the following test on Chrome, Firefox, Opera and all of them produced the same output: "aaa" remains small, "bbb" grows big. For some reason, normalized scientific notation can not be used to represent font-size values outside of SVG blocks. (Not sure if it is a bug or not.)

<html>
   <body>
    <div style="font-size:8E+2%">aaa</div>
  
    <svg xmlns="http://www.w3.org/2000/svg">
     <text y="150" font-size="8E+2%">bbb</text>
    </svg>
   </body>
</html>

I could only extend the previous test to non SVG values by using standard decimal notation.

<html>
  <body>
    <div style="font-size: 900000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000%;">aaa    </div>

    <svg xmlns="http://www.w3.org/2000/svg">
       <text y="150" font-size="8E+2%">bbb</text>
    </svg>
  </body>
</html>

[Martin Hodovan]

Is the test OK like this?

[Benjamin Poulain]

(In reply to comment #15)
> Is the test OK like this?

Yep, that looks ok. :)

[Martin Hodovan]

Created attachment 224422 [details]
Proposed patch

There was an extra new line character at the end of the expected file, that's why the test failed on Mac. I also fixed the indentation.

[Build Bot]

Comment on attachment 224422 [details]
Proposed patch

Attachment 224422 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5792670509170688

New failing tests:
fast/css/parsing-infinite-floating-value.html

[Build Bot]

Created attachment 224429 [details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 224422 [details]
Proposed patch

Attachment 224422 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5247733648588800

New failing tests:
fast/css/parsing-infinite-floating-value.html

[Build Bot]

Created attachment 224436 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 224422 [details]
Proposed patch

Attachment 224422 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5057601754628096

New failing tests:
fast/css/parsing-infinite-floating-value.html

[Build Bot]

Created attachment 224440 [details]
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-05  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Martin Hodovan]

Created attachment 224638 [details]
Proposed patch

[Martin Hodovan]

Created attachment 225072 [details]
Proposed patch

I have relocated the infinity check to CSSValuePool::createValue(), I think this is the function where it really belongs to. This way we dont have to handle the SVG and non-SVG cases separately.

[Martin Hodovan]

Could someone take a look at this?

[Darin Adler]

Comment on attachment 225072 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=225072&action=review

> Source/WebCore/css/CSSValuePool.cpp:93
> +    if (!std::isfinite(value))
> +        return createIdentifierValue(CSSValueID::CSSValueInfinite);

It’s fine to change infinity to infinite, but not good to change NaN to infinite. I suggest using std::isinf here instead of std::isfinite. Also, is it OK to change negative infinity to positive infinity?

[Darin Adler]

Comment on attachment 225072 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=225072&action=review

>> Source/WebCore/css/CSSValuePool.cpp:93
>> +        return createIdentifierValue(CSSValueID::CSSValueInfinite);
> 
> It’s fine to change infinity to infinite, but not good to change NaN to infinite. I suggest using std::isinf here instead of std::isfinite. Also, is it OK to change negative infinity to positive infinity?

Testing ought to cover more than just parsing. Which properties support "infinite" as a value? Is it correct for these large numbers to act the same as "infinite" for those? What about properties that don’t support "infinite"? Is it OK for the large number to act as if it was an illegal value?

[Martin Hodovan]

I believe that no infinite values should be supported in CSS at all. Properties that are
expecting numeric values (most of them are listed in CSSParser::isSimpleLengthPropertyID
like FontSize, (Min/Max of) Height/Width, Bottom/Left/Right/Top-Margin/Padding, etc.) 
do not make any sense in infinite cases, regardless it is a positive or negative value,
and since there is no sensible way to present them, I think we should just ignore them all.
Maybe I should use CSSValueID::CSSValueInvalid instead of CSSValueID::CSSValueInfinite. (?)
The standard only says "UAs should support reasonably useful ranges and precisions."
I am a little confused, how do you mean that testing ought to cover more than just parsing?
I was using the parsed values as font-size and line-height property values.

[Martin Hodovan]

Am I wrong about it?

[Darin Adler]

(In reply to comment #29)
> I believe that no infinite values should be supported in CSS at all.

Makes sense to me.

> Maybe I should use CSSValueID::CSSValueInvalid instead of CSSValueID::CSSValueInfinite.

Why do we even have CSSValueInfinite?

[Martin Hodovan]

Created attachment 227043 [details]
Proposed patch

[Martin Hodovan]

(In reply to comment #31)
I replaced !std::isfinite() with std::isinf() and CSSValueID::CSSValueInfinite
with CSSValueID::CSSValueInvalid. I also changed the name of the test case,
because it really was a little misleading (this patch is not parsing-related).

> Why do we even have CSSValueInfinite?
CSSValueInfinite is used by the -webkit-marquee-repetition property,
when we want to set the number of repeats to infinite explicitly, like 
"-webkit-marquee-repetition: infinite;".

[WebKit Commit Bot]

Comment on attachment 227043 [details]
Proposed patch

Rejecting attachment 227043 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 227043, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

/Volumes/Data/EWS/WebKit/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://webkit-queues.appspot.com/results/6731491404939264

[Darin Adler]

Comment on attachment 227043 [details]
Proposed patch

Clearing flags on attachment: 227043

Committed r166114: <http://trac.webkit.org/changeset/166114>

[Darin Adler]

All reviewed patches have been landed.  Closing bug.