Bug 131023

Summary: [CSS Grid Layout] Clamping the number of repetitions in repeat()
Product: WebKit Reporter: Javier Fernandez <jfernandez>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, darin, esprehn+autocc, glenn, gyuyoung.kim, hyatt, jfernandez, kling, macpherson, menard, rego, svillar
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 60731    
Attachments:
Description Flags
Patch
none
Patch none

Javier Fernandez
Reported 2014-04-01 02:20:19 PDT
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
Attachments
Patch (4.33 KB, patch)
2014-04-25 04:29 PDT, Javier Fernandez 		no flags
DetailsFormatted DiffDiff
Patch (4.73 KB, patch)
2014-05-01 09:28 PDT, Javier Fernandez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Javier Fernandez
Comment 1 2014-04-25 04:29:12 PDT
Created attachment 230164 [details] Patch
Brent Fulgham
Comment 2 2014-04-25 09:54:06 PDT
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 3 2014-04-25 09:57:13 PDT
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
Comment 4 2014-05-01 09:15:41 PDT
(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
Comment 5 2014-05-01 09:28:34 PDT
Created attachment 230584 [details] Patch Besides applying suggested changes, I've modified the tests to do also some boundary testing.
Brent Fulgham
Comment 6 2014-05-01 10:19:16 PDT
Comment on attachment 230584 [details] Patch r=me
WebKit Commit Bot
Comment 7 2014-05-01 10:49:07 PDT
Comment on attachment 230584 [details] Patch Clearing flags on attachment: 230584 Committed r168108: <http://trac.webkit.org/changeset/168108>
WebKit Commit Bot
Comment 8 2014-05-01 10:49:13 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.