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.
Created attachment 130249 [details] Patch
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. :/
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?
...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!
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 ;-)
Created attachment 133724 [details] Patch
This looks reasonable to me.
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.
Created attachment 167168 [details] Patch for landing
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
(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).
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.
Comment on attachment 167168 [details] Patch for landing Clearing flags on attachment: 167168 Committed r130437: <http://trac.webkit.org/changeset/130437>
All reviewed patches have been landed. Closing bug.
(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?
(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