RESOLVED FIXED 80360
1ex should equal .5em when the font has no x-height metric 
https://bugs.webkit.org/show_bug.cgi?id=80360
Summary 1ex should equal .5em when the font has no x-height metric
Tab Atkins
Reported 2012-03-05 17:24:42 PST
Testcase ======== <!doctype html> <svg xmlns = 'http://www.w3.org/2000/svg' style='display:none'> <defs> <font id="Font1"> <font-face font-family="block" /> </font> </defs> </svg> <style> .em { height: .5em; width: .5em; } .ex { height: 1ex; width: 1ex; } .cover { background: green; } .back { background: red; } .one { left: 10px; } .two { left: 70px; } #test > div { font-family: block; font-size: 100px; position: absolute; top: 10px; } </style> <div id='test'> <div class='back em one'></div> <div class='cover ex one'></div> <div class='back ex two'></div> <div class='cover em two'></div> </div> Expected result =============== Because the SVG font doesn't define an x-height metric, the 'ex' unit for elements that use it should be sized to half the 'em' unit. Actual result ============= The 'ex' unit is sized to 0. This also occurs with TrueType fonts that have been modified to not have an x-height metric.
Attachments
Patch (8.67 KB, patch)
2012-03-05 17:41 PST, Tab Atkins 		no flags
DetailsFormatted DiffDiff
Patch (7.80 KB, patch)
2012-03-25 19:48 PDT, David Barr 		no flags
DetailsFormatted DiffDiff
Patch for landing (7.81 KB, patch)
2012-10-04 13:44 PDT, Tab Atkins 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Tab Atkins
Comment 1 2012-03-05 17:41:55 PST
Created attachment 130249 [details] Patch
Tab Atkins
Comment 2 2012-03-05 17:49:01 PST
The above patch definitely works with cr-linux, but I'm unsure of what the other platforms return from their various font libraries. I need to see bot results before I can be sure that my patch is complete. :/
Tim Horton
Comment 3 2012-03-06 14:04:16 PST
Comment on attachment 130249 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=130249&action=review > Source/WebCore/platform/wx/wxcode/win/fontprops.cpp:61 > - m_xHeight = m_ascent * 0.56f; // Best guess for xHeight for non-Truetype fonts. > - m_lineGap = lroundf(tm.tmExternalLeading); > + m_xHeight = m_ascent * 0.56f; // Best guess for xHeight for non-Truetype fonts.0;lroundf(tm.tmExternalLeading); Did you mean to remove the assignment to m_lineGap and put its associated computation at the end of a comment?
Tab Atkins
Comment 4 2012-03-06 14:54:28 PST
...huh. I *thought* that comment looked weird. I have no idea how I managed to accidentally do that. I'm about to be OOO for the week; I'll fix that and resubmit when I get back next week. Thanks!
Hajime Morrita
Comment 5 2012-03-15 01:54:19 PDT
Comment on attachment 130249 [details] Patch R-minusing based on Tim's comment to shorten the review queue. I'm looking for your next patch ;-)
David Barr
Comment 6 2012-03-25 19:48:43 PDT
Created attachment 133724 [details] Patch
Eric Seidel (no email)
Comment 7 2012-04-19 15:26:11 PDT
This looks reasonable to me.
Eric Seidel (no email)
Comment 8 2012-04-19 15:26:58 PDT
Comment on attachment 133724 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133724&action=review > Source/WebCore/platform/graphics/skia/SimpleFontDataSkia.cpp:109 > + m_fontMetrics.setHasXHeight(false); You might want to explain this line here with a comment.
Tab Atkins
Comment 9 2012-10-04 13:44:01 PDT
Created attachment 167168 [details] Patch for landing
Eric Seidel (no email)
Comment 10 2012-10-04 13:48:51 PDT
Comment on attachment 167168 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=167168&action=review > Source/WebCore/platform/graphics/FontMetrics.h:133 > + bool m_hasXHeight; Do we care about the size of this object such that we should be stealing a bit from another member instead of adding a whole int/byte/word/whatever
Tab Atkins
Comment 11 2012-10-04 13:51:27 PDT
(In reply to comment #10) > (From update of attachment 167168 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=167168&action=review > > > Source/WebCore/platform/graphics/FontMetrics.h:133 > > + bool m_hasXHeight; > > Do we care about the size of this object such that we should be stealing a bit from another member instead of adding a whole int/byte/word/whatever I have no idea! We shouldn't have many FontMetrics objects in a page, though, so I doubt it's much of an issue (unless I'm wrong).
Eric Seidel (no email)
Comment 12 2012-10-04 13:56:36 PDT
Comment on attachment 167168 [details] Patch for landing We'll have one for every instance of DataRef<StyleInheritedData> inherited; each element which has a different value for an inherited property than it's parent will have a unique StyleInheritedData object. I suspect this isn't a huge deal. But I do suspect it will have a small memory effect. The solution here is for someone to look at the overall memory usage of the style tree and make it smaller if needed. I don't think that needs to hold up this patch however.
WebKit Review Bot
Comment 13 2012-10-04 17:35:47 PDT
Comment on attachment 167168 [details] Patch for landing Clearing flags on attachment: 167168 Committed r130437: <http://trac.webkit.org/changeset/130437>
WebKit Review Bot
Comment 14 2012-10-04 17:35:51 PDT
All reviewed patches have been landed. Closing bug.
Darin Adler
Comment 15 2012-10-05 17:08:59 PDT
(In reply to comment #0) > Because the SVG font doesn't define an x-height metric, the 'ex' unit for elements that use it should be sized to half the 'em' unit. Where does that idea come from? A specification?
Tab Atkins
Comment 16 2012-10-05 18:33:23 PDT
(In reply to comment #15) > (In reply to comment #0) > > Because the SVG font doesn't define an x-height metric, the 'ex' unit for elements that use it should be sized to half the 'em' unit. > > Where does that idea come from? A specification? Yes: http://dev.w3.org/csswg/css3-values/#ex-unit
Note You need to log in before you can comment on or make changes to this bug.