SVG doesn't recognize ch units.

SVG doesn't recognize ch units.

Adds support for the 'ch' unit in SVG <length> values.

BUG=368598
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=193591

[Shanmuga Pandi, 2015-04-09 14:10:05]

shanmuga.m@samsung.com changed reviewers:
+ ed@opera.com, pdr@chromium.org, timloh@chromium.org

[Shanmuga Pandi, 2015-04-09 14:10:05]

Please review it.

[Erik Dahlström (inactive), 2015-04-10 08:43:18]

https://codereview.chromium.org/1073803002/diff/1/LayoutTests/svg/repaint/svg...
File LayoutTests/svg/repaint/svg-length-ch-unit-font-size-change.html (right):

https://codereview.chromium.org/1073803002/diff/1/LayoutTests/svg/repaint/svg...
LayoutTests/svg/repaint/svg-length-ch-unit-font-size-change.html:19: <div
id="rootdiv" style="font-size:10px; font-family:serif">
It would be better to use a particular font instead of a generic family. The
Ahem font is typically a good fit for these kind of things. This comment applies
to all the tests using the 'ch' unit.

https://codereview.chromium.org/1073803002/diff/1/Source/core/svg/SVGLengthCo...
File Source/core/svg/SVGLengthContext.cpp (right):

https://codereview.chromium.org/1073803002/diff/1/Source/core/svg/SVGLengthCo...
Source/core/svg/SVGLengthContext.cpp:316: // Use of ceil allows a pixel match to
the W3Cs expected output of coords-units-03-b.svg
This test doesn't use ch units, so this comment doesn't apply.

Better to be consistent with EX units, or to drop the ceilf from both of them?

https://codereview.chromium.org/1073803002/diff/1/Source/core/svg/SVGLengthCo...
Source/core/svg/SVGLengthContext.cpp:331: // Use of ceil allows a pixel match to
the W3Cs expected output of coords-units-03-b.svg
Same as above.

[Shanmuga Pandi, 2015-04-10 11:46:16]

https://codereview.chromium.org/1073803002/diff/1/LayoutTests/svg/repaint/svg...
File LayoutTests/svg/repaint/svg-length-ch-unit-font-size-change.html (right):

https://codereview.chromium.org/1073803002/diff/1/LayoutTests/svg/repaint/svg...
LayoutTests/svg/repaint/svg-length-ch-unit-font-size-change.html:19: <div
id="rootdiv" style="font-size:10px; font-family:serif">
On 2015/04/10 08:43:18, Erik Dahlström wrote:
> It would be better to use a particular font instead of a generic family. The
> Ahem font is typically a good fit for these kind of things. This comment
applies
> to all the tests using the 'ch' unit.

Done.

https://codereview.chromium.org/1073803002/diff/1/Source/core/svg/SVGLengthCo...
File Source/core/svg/SVGLengthContext.cpp (right):

https://codereview.chromium.org/1073803002/diff/1/Source/core/svg/SVGLengthCo...
Source/core/svg/SVGLengthContext.cpp:316: // Use of ceil allows a pixel match to
the W3Cs expected output of coords-units-03-b.svg
On 2015/04/10 08:43:18, Erik Dahlström wrote:
> This test doesn't use ch units, so this comment doesn't apply.
> 
> Better to be consistent with EX units, or to drop the ceilf from both of them?

Removed comment and ceilf

https://codereview.chromium.org/1073803002/diff/1/Source/core/svg/SVGLengthCo...
Source/core/svg/SVGLengthContext.cpp:331: // Use of ceil allows a pixel match to
the W3Cs expected output of coords-units-03-b.svg
On 2015/04/10 08:43:18, Erik Dahlström wrote:
> Same as above.

Done.

[fs, 2015-04-10 12:17:19]

fs@opera.com changed reviewers:
+ fs@opera.com

[fs, 2015-04-10 12:17:24]

https://codereview.chromium.org/1073803002/diff/20001/Source/core/svg/SVGLeng...
File Source/core/svg/SVGLength.cpp (right):

https://codereview.chromium.org/1073803002/diff/20001/Source/core/svg/SVGLeng...
Source/core/svg/SVGLength.cpp:108: } else if (firstChar == 'c' && secondChar ==
'h') {
Merge this with the other case starting with 'c' (like for 'em'/'ex' above).

[Shanmuga Pandi, 2015-04-10 13:09:19]

https://codereview.chromium.org/1073803002/diff/20001/Source/core/svg/SVGLeng...
File Source/core/svg/SVGLength.cpp (right):

https://codereview.chromium.org/1073803002/diff/20001/Source/core/svg/SVGLeng...
Source/core/svg/SVGLength.cpp:108: } else if (firstChar == 'c' && secondChar ==
'h') {
On 2015/04/10 12:17:24, fs wrote:
> Merge this with the other case starting with 'c' (like for 'em'/'ex' above).

Done.

[Erik Dahlström (inactive), 2015-04-10 13:48:43]

lgtm

[Shanmuga Pandi, 2015-04-10 13:51:04]

The CQ bit was checked by shanmuga.m@samsung.com

[commit-bot: I haz the power, 2015-04-10 13:51:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1073803002/40001

[commit-bot: I haz the power, 2015-04-10 14:00:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-10 14:00:29]

Try jobs failed on following builders:
  blink_presubmit on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/3...)

[fs, 2015-04-10 14:28:57]

(rs-)lgtm

[Shanmuga Pandi, 2015-04-11 06:02:50]

The CQ bit was checked by shanmuga.m@samsung.com

[commit-bot: I haz the power, 2015-04-11 06:03:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1073803002/40001

[commit-bot: I haz the power, 2015-04-11 06:06:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-11 06:07:00]

Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/51453)

[Shanmuga Pandi, 2015-04-11 08:50:56]

The CQ bit was unchecked by shanmuga.m@samsung.com

[Shanmuga Pandi, 2015-04-11 08:50:57]

The CQ bit was checked by shanmuga.m@samsung.com

[Shanmuga Pandi, 2015-04-11 08:50:57]

The patchset sent to the CQ was uploaded after l-g-t-m from ed@opera.com,
fs@opera.com
Link to the patchset: https://codereview.chromium.org/1073803002/#ps60001
(title: "Rebased")

[commit-bot: I haz the power, 2015-04-11 08:51:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1073803002/60001

[commit-bot: I haz the power, 2015-04-11 11:04:12]

Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=193591