Bug 127720

Summary: CSS 2.2: Support scientific notations for all numbers in CSS
Product: WebKit Reporter: Dirk Schulze <krit>
Component: CSSAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: anvaka, buildbot, commit-queue, dbates, dino, eoconnor, esprehn+autocc, glenn, gyuyoung.kim, hyatt, kling, koivisto, macpherson, menard, michael.herzog, mmaxfield, pvp.ruler, rniwa, simon.fraser, syoichi, webkit-bug-importer
Priority: P2 Keywords: InRadar, WebExposed
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 162024    
Bug Blocks:    
Attachments:
Description Flags
Patch
dbates: review-, buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2
none
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion
none
WIP
none
Reduction none

Description Dirk Schulze 2014-01-27 15:51:34 PST
CSS 2.2 will support scientific notations for all number values. We support scientific numbers for SVG in the CSSParser. We should do for all values now:

See line 11711 in CSSParser.cpp:

#if ENABLE(SVG)
        // Use SVG parser for numbers on SVG presentation attributes.
        if (m_context.mode == SVGAttributeMode) {
            // We need to take care of units like 'em' or 'ex'.
            SrcCharacterType* character = currentCharacter<SrcCharacterType>();
            if (isASCIIAlphaCaselessEqual(*character, 'e')) {
                ASSERT(character - tokenStart<SrcCharacterType>() > 0);
                ++character;
                if (*character == '-' || *character == '+' || isASCIIDigit(*character)) {
                    ++character;
                    while (isASCIIDigit(*character))
                        ++character;
                    // Use FLOATTOKEN if the string contains exponents.
                    dotSeen = true;
                    currentCharacter<SrcCharacterType>() = character;
                }
            }
            if (!parseSVGNumber(tokenStart<SrcCharacterType>(), character - tokenStart<SrcCharacterType>(), yylval->number))
                break;
        } else
#endif
            yylval->number = charactersToDouble(tokenStart<SrcCharacterType>(), currentCharacter<SrcCharacterType>() - tokenStart<SrcCharacterType>());
Comment 2 Dirk Schulze 2014-01-28 03:01:10 PST
(In reply to comment #0)
> CSS 2.2 will support scientific notations for all number values. We support scientific numbers for SVG in the CSSParser. We should do for all values now:
> 
> See line 11711 in CSSParser.cpp:
> 
> #if ENABLE(SVG)
>         // Use SVG parser for numbers on SVG presentation attributes.
>         if (m_context.mode == SVGAttributeMode) {
>             // We need to take care of units like 'em' or 'ex'.
>             SrcCharacterType* character = currentCharacter<SrcCharacterType>();
>             if (isASCIIAlphaCaselessEqual(*character, 'e')) {
>                 ASSERT(character - tokenStart<SrcCharacterType>() > 0);
>                 ++character;
>                 if (*character == '-' || *character == '+' || isASCIIDigit(*character)) {
>                     ++character;
>                     while (isASCIIDigit(*character))
>                         ++character;
>                     // Use FLOATTOKEN if the string contains exponents.
>                     dotSeen = true;
>                     currentCharacter<SrcCharacterType>() = character;
>                 }
>             }
>             if (!parseSVGNumber(tokenStart<SrcCharacterType>(), character - tokenStart<SrcCharacterType>(), yylval->number))
>                 break;
>         } else
> #endif
>             yylval->number = charactersToDouble(tokenStart<SrcCharacterType>(), currentCharacter<SrcCharacterType>() - tokenStart<SrcCharacterType>());

It is of course easy to remove the if (m_context.mode == SVGAttributeMode) check, but I don't know how it impacts performance. This might need to be checked. In SVG (where we use this code all of the time) the code never stood out on performance measurements though.
Comment 3 Dirk Schulze 2014-03-20 15:47:25 PDT
Created attachment 227348 [details]
Patch
Comment 4 Build Bot 2014-03-20 17:49:30 PDT
Comment on attachment 227348 [details]
Patch

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

New failing tests:
media/controls-strict.html
media/audio-controls-rendering.html
fast/filter-image/parse-filter-image.html
media/controls-without-preload.html
fast/css/round-trip-values.html
css3/filters/filter-property-computed-style.html
css3/filters/filter-property-parsing.html
fast/css/large-value-csstext.html
Comment 5 Build Bot 2014-03-20 17:49:36 PDT
Created attachment 227362 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 6 Build Bot 2014-03-20 19:02:21 PDT
Comment on attachment 227348 [details]
Patch

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

New failing tests:
media/controls-strict.html
media/audio-controls-rendering.html
fast/filter-image/parse-filter-image.html
media/controls-without-preload.html
fast/css/round-trip-values.html
http/tests/media/track/track-webvtt-slow-loading.html
css3/filters/filter-property-computed-style.html
media/controls-after-reload.html
css3/filters/filter-property-parsing.html
fast/css/large-value-csstext.html
Comment 7 Build Bot 2014-03-20 19:02:31 PDT
Created attachment 227368 [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
Comment 8 Daniel Bates 2016-04-23 11:58:46 PDT
Comment on attachment 227348 [details]
Patch

r- given that this patch caused some changes to how CSS floating point numbers are parsed given that we always use parseSVGNumber() regardless of the mode we are in.
Comment 9 Myles C. Maxfield 2016-05-11 11:31:58 PDT
(In reply to comment #8)
> Comment on attachment 227348 [details]
> Patch
> 
> r- given that this patch caused some changes to how CSS floating point
> numbers are parsed given that we always use parseSVGNumber() regardless of
> the mode we are in.

I don't understand this rationale. The spec has changed (and both Firefox and Chrome have updated to match it) to specify that CSS floating point numbers ::should:: parsed regardless of the mode we are in.
Comment 10 Myles C. Maxfield 2016-05-11 12:18:16 PDT
Created attachment 278652 [details]
WIP
Comment 11 Myles C. Maxfield 2016-05-11 12:19:16 PDT
Created attachment 278653 [details]
Reduction
Comment 12 Myles C. Maxfield 2016-05-11 12:19:41 PDT
*** Bug 154506 has been marked as a duplicate of this bug. ***
Comment 13 Myles C. Maxfield 2016-05-11 13:09:19 PDT
<rdar://problem/25511333>
Comment 14 Michael Herzog 2016-09-04 02:41:40 PDT
Is somebody working on this bug? I always need an annoying workaround for Safari when developing 3D web applications with three.js (see https://github.com/mrdoob/three.js/issues/7802) or with plain CSS 3D transforms (using matrix3d).
Comment 15 pvp.ruler 2016-09-09 15:24:21 PDT
Indeed, a fix would be nice!
Comment 16 Simon Fraser (smfr) 2016-09-13 12:56:54 PDT
This will be fixed in the new CSS parser (bug 161916 and others).
Comment 17 Michael Herzog 2016-09-15 01:07:10 PDT
The reduction test still fails with r205937.
Comment 18 Simon Fraser (smfr) 2016-09-15 09:59:18 PDT
I didn't mean that commit would fix it, only that it will get fixed eventually when the new CSS parser is enabled.
Comment 19 Michael Herzog 2016-09-15 11:02:24 PDT
Oh, i see ;)

So...when gets the new CSS parser enabled? Is there something like a schedule or a target release for this feature? I'd like to verify the implementation with the given reduction test as soon as it's available in webkit nightly.
Comment 20 Dean Jackson 2016-09-15 12:44:40 PDT
Maybe in a month or so. Not within the next few weeks. There are a lot of little pieces to get going.
Comment 21 Michael Herzog 2016-11-10 10:00:14 PST
Sry for asking this but when can we expect a status update for this issue?
Comment 22 Dean Jackson 2016-11-10 11:32:53 PST
This is now fixed. However, it's just waiting on the new CSS parser to be enabled by default. This should happen in the coming days or week.
Comment 23 Michael Herzog 2016-12-09 14:25:01 PST
Thanks for fixing this!