[Webkit-unassigned] [Bug 129041] [CSS Grid Layout] Update named grid lines syntax to the last version of the specs

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=129041


Sergio Villar Senin <svillar at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #226127|review?                     |review-
               Flag|                            |




--- Comment #12 from Sergio Villar Senin <svillar at igalia.com>  2014-03-07 08:31:01 PST ---
(From update of attachment 226127)
View in context: https://bugs.webkit.org/attachment.cgi?id=226127&action=review

Getting better, but r- due to missing guards and mistakes in the tests.

> Source/WebCore/ChangeLog:13
> +        Updated tests to match the new grid line-name syntax.

the new <grid-line>

> Source/WebCore/css/CSSGrammar.y.in:278
> +%type <value> track_names_list

Maybe we could guard the grammar code. The ident_list is general purpose and might be reused in the future by some other stuff but it would be a matter of moving it out of the guards.

> Source/WebCore/css/CSSParser.cpp:4894
> +        lineNames->append(lineName.release());

You can join these two in a single line.

> Source/WebCore/css/CSSParser.cpp:4932
> +        // This will handle the trailing <ident>* in the grammar.

<custom-ident>

> Source/WebCore/css/CSSParser.cpp:4957
> +    // Handle leading <ident>*.

Ditto.

> Source/WebCore/css/CSSParser.cpp:4973
> +        // This takes care of any trailing <ident>* in the grammar.

Ditto.

> Source/WebCore/css/CSSParser.h:570
> +    Vector<CSSParserValueList*> m_floatingValueLists;

This is not needed.

> Source/WebCore/css/CSSValue.cpp:43
> +#include "CSSGridLineNamesValue.h"

ENABLE(CSS_GRID_LAYOUT)

> Source/WebCore/css/StyleResolver.cpp:44
> +#include "CSSGridLineNamesValue.h"

ENABLE(CSS_GRID_LAYOUT)

> LayoutTests/fast/css-grid-layout/grid-item-negative-position-resolution.html:102
> +    -webkit-grid-row: 1;

Good catch!!! but you forgot the "middle".

> LayoutTests/fast/css-grid-layout/named-grid-line-get-set-expected.txt:67
> +FAIL getComputedStyle(element, '').getPropertyValue('-webkit-grid-template-columns') should be (first nav) 0px (nav) 0px (last). Was (nav first) 0px (nav) 0px (last).

Those FAIL's are not correct. The problem is that the line names are listed in the wrong order. We have bug 127837 for that. In the meantime just use the unordered version.

> LayoutTests/fast/css-grid-layout/non-named-grid-line-get-set-expected.txt:17
> +FAIL window.getComputedStyle(gridWithPercentageSameStringMultipleTimes, '').getPropertyValue('-webkit-grid-template-columns') should be (first nav) 10% (nav) 15% (last). Was (nav first) 10% (nav) 15% (last).

Same issue here with the order of the line names.

> LayoutTests/fast/css-grid-layout/non-named-grid-line-get-set-expected.txt:61
> +FAIL getComputedStyle(element, '').getPropertyValue('-webkit-grid-template-columns') should be (first nav) minmax(-webkit-min-content, -webkit-max-content) (nav) auto (last). Was (nav first) minmax(-webkit-min-content, -webkit-max-content) (nav) auto (last).

Ditto.

> LayoutTests/fast/css-grid-layout/non-named-grid-line-get-set.html:92
> +    element.style.webkitGridTemplateColumns = "(foo 'bar)";

extra "'" here

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.