Bug 131023 – [CSS Grid Layout] Clamping the number of repetitions in repeat()

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Javier Fernandez]

The ED suggests now to be able to clamp the number of repetitions when using the repeat() function, taking precautions about excessive memory usage.

See the mailing list thread for more details:

  - http://lists.w3.org/Archives/Public/www-style/2014Mar/0553.html

[Javier Fernandez]

Created attachment 230164 [details]
Patch

[Brent Fulgham]

Comment on attachment 230164 [details]
Patch

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

> Source/WebCore/css/CSSParser.cpp:4930
> +        repetitions = MAX_REPETITIONS;

Should there be any kind of feedback to the inspector when this happens?

[Brent Fulgham]

Comment on attachment 230164 [details]
Patch

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

This patch seems fine overall. Please make make the MAX_REPETITIONS name more descriptive when you land.

>> Source/WebCore/css/CSSParser.cpp:4930
>> +        repetitions = MAX_REPETITIONS;
> 
> Should there be any kind of feedback to the inspector when this happens?

Also: I think you should reference the specification or draft that specifies this limit, and call the variable "MAX_GRID_TRACK_REPETITIONS".

[Javier Fernandez]

(In reply to comment #2)
> (From update of attachment 230164 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=230164&action=review
> 
> > Source/WebCore/css/CSSParser.cpp:4930
> > +        repetitions = MAX_REPETITIONS;
> 
> Should there be any kind of feedback to the inspector when this happens?

umm, good question. I think the limit of 10000 is high enough to now be reached in real webs, so the clamping operation won't be very frequent. On the other hand, when using values higher than 10000 it wont be easy to realize the value was clamped watching at the computed value. So, I'm not sure whether informing the inspector is worth or not. 

Personally, I think that the case of clamping values is so rare that I think it's not worth to deal with the inspector.

[Javier Fernandez]

Created attachment 230584 [details]
Patch

Besides applying suggested changes, I've modified the tests to do also some boundary testing.

[Brent Fulgham]

Comment on attachment 230584 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 230584 [details]
Patch

Clearing flags on attachment: 230584

Committed r168108: <http://trac.webkit.org/changeset/168108>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.