Bug 47157 - CSS 2.1 failure: background-position-001, background-position-002
Summary: CSS 2.1 failure: background-position-001, background-position-002
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Dave Hyatt
URL: http://test.csswg.org/suites/css2.1/2...
Keywords:
: 47158 (view as bug list)
Depends on: 52122
Blocks: 47141
  Show dependency treegraph
 
Reported: 2010-10-04 21:50 PDT by Simon Fraser (smfr)
Modified: 2011-03-10 13:29 PST (History)
4 users (show)

See Also:


Attachments
background-position-001.htm (1.24 KB, text/html)
2010-10-04 21:50 PDT, Simon Fraser (smfr)
no flags Details
Patch (274.31 KB, patch)
2011-03-10 12:58 PST, Dave Hyatt
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2010-10-04 21:50:25 PDT
Created attachment 69752 [details]
background-position-001.htm

These tests fail:

html4/background-position-001	fail
html4/background-position-002	fail
Comment 1 Simon Fraser (smfr) 2011-01-08 12:25:38 PST
These tests rely on 'ex' units.
Comment 2 Simon Fraser (smfr) 2011-01-08 12:52:42 PST
We're getting an xHeight of 20 for the Ahem font at 20px, but it seems that the xHeight should be 80% of this <http://hixie.ch/resources/fonts/>

CTFontGetBoundingRectsForGlyphs() returns a rect of 0,-4, 20x20, which
 boundingBox.setY(-boundingBox.bottom());
converts to  0,-16, 20x20
which seems wrong.
Comment 3 mitz 2011-01-08 13:03:07 PST
Right, on Mac OS X, WebKit prefers the height of the lowercase x glyph over the x-height encoded in the font.
Comment 4 Simon Fraser (smfr) 2011-01-08 13:08:33 PST
Ah, the culprit is:

        // Use the maximum of either width or height because "x" is nearly square
        // and web pages that foolishly use this metric for width will be laid out
        // poorly if we return an accurate height. Classic case is Times 13 point,
        // which has an "x" that is 7x6 pixels.
        m_xHeight = static_cast<float>(max(CGRectGetMaxX(xBox), -CGRectGetMinY(xBox)));

Maybe we should make an exception for the Ahem font?
Comment 5 Simon Fraser (smfr) 2011-01-08 13:16:47 PST
The tests covered by bug 47158 also rely on ex units.
Comment 6 Simon Fraser (smfr) 2011-03-09 17:52:25 PST
Fixed by http://trac.webkit.org/changeset/80662 ?
Comment 7 Dave Hyatt 2011-03-10 12:56:53 PST
*** Bug 47158 has been marked as a duplicate of this bug. ***
Comment 8 Dave Hyatt 2011-03-10 12:57:54 PST
No, not fixed by r80662.  Actual patch coming shortly.
Comment 9 Dave Hyatt 2011-03-10 12:58:27 PST
Created attachment 85379 [details]
Patch
Comment 10 Simon Fraser (smfr) 2011-03-10 13:04:27 PST
Comment on attachment 85379 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=85379&action=review

> Source/WebCore/ChangeLog:5
> +        https://bugs.webkit.org/show_bug.cgi?id=47157, CSS2.1 test failures because the ex unit is broken

We normally put the bug URL on its own line.

> LayoutTests/accessibility/image-map-title-causes-crash-expected.txt:-10
> - 1
> -Requesting the title of an AccessibilityImageMapLink can cause a crash when the map's area element has been removed.
> -
> -On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
> -
> -
> -PASS successfullyParsed is true
> -
> -TEST COMPLETE
> -

Why did this result go away?
Comment 11 Dave Hyatt 2011-03-10 13:29:37 PST
Fixed in r80755.