Bug 80360

Summary:

1ex should equal .5em when the font has no x-height metric

Product:

WebKit

Reporter:

Tab Atkins <tabatkins>

Component:

CSS

Assignee:

Tab Atkins <tabatkins>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ap, cmarcelo, darin, davidbarr, efidler, eric, hyatt, macpherson, menard, mitz, senorblanco, thorton, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
Patch	none
Patch for landing	none

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	Details Formatted Diff Diff
Patch (7.80 KB, patch) 2012-03-25 19:48 PDT, David Barr	no flags	Details Formatted Diff Diff
Patch for landing (7.81 KB, patch) 2012-10-04 13:44 PDT, Tab Atkins	no flags	Details Formatted Diff Diff
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.