| Summary: | Introduce templated compute length class in CSSStyleApplyProperty, and use to handle several CSS properties. | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Luke Macpherson <macpherson> | ||||||||||
| Component: | New Bugs | Assignee: | Luke Macpherson <macpherson> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | dglazkov, eric, macpherson, webkit.review.bot | ||||||||||
| Priority: | P2 | ||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Luke Macpherson
2011-06-19 18:40:43 PDT
Created attachment 97729 [details]
Patch
Note that this patch causes negative border widths to clamp to zero, which is equivalent to the current behavior of ignoring negative values and using the default border width of zero, but avoids the conversions between short and unsigned short and additional checks for negative computed lengths that were present in the existing code. Created attachment 97731 [details]
Patch
Comment on attachment 97731 [details] Patch Attachment 97731 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/8911342 Created attachment 97738 [details]
Patch
Comment on attachment 97738 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=97738&action=review > Source/WebCore/css/CSSStyleApplyProperty.cpp:395 > +template <typename T, > + ComputeLengthNormal normalEnabled = NormalDisabled, > + ComputeLengthThickness thicknessEnabled = ThicknessDisabled, > + ComputeLengthSVGZoom svgZoomEnabled = SVGZoomDisabled> Really? There isn't a better way to do this than 3 independent bools? (In reply to comment #6) > (From update of attachment 97738 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=97738&action=review > > > Source/WebCore/css/CSSStyleApplyProperty.cpp:395 > > +template <typename T, > > + ComputeLengthNormal normalEnabled = NormalDisabled, > > + ComputeLengthThickness thicknessEnabled = ThicknessDisabled, > > + ComputeLengthSVGZoom svgZoomEnabled = SVGZoomDisabled> > > Really? There isn't a better way to do this than 3 independent bools? Well, feel free to suggest one. I think it makes very good sense to do it that way, since they are orthogonal properties that you might want to switch on and off for individual CSS properties depending on what the spec allows. Ping. Patch still applies and builds cleanly. Comment on attachment 97738 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=97738&action=review > Source/WebCore/ChangeLog:12 > + Add computeLenght function that returns unsigned short. computeLength Created attachment 100765 [details]
Patch
Comment on attachment 100765 [details]
Patch
you can just use webkit-patch land-safely next time. It will transfer the r+ from the previous patch and cq+ the new one.
Comment on attachment 100765 [details] Patch Clearing flags on attachment: 100765 Committed r91038: <http://trac.webkit.org/changeset/91038> All reviewed patches have been landed. Closing bug. |