581087 - REFTEST TEST-UNEXPECTED-FAIL | file:///c:/talos-slave/mozilla-central-win7-opt-u-reftest-d2d/build/reftest/tests/layout/reftests/forms/input-text-size-1.html |

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

One version has a caret the other does not.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

See bug 519361

Comment 2

[Bas Schouten (:bas.schouten)]

Would either of you guys be willing to look at this? The textbox appears to be
wider on D2D than without. Allowing one extra character to slip in, it's not
the caret, it's a '|' character.

Comment 3

[Jonathan Kew [:jfkthame]]

This happens because the DirectWrite font metrics APIs do not provide the maxAdvance and averageCharWidth values (as GDI did), and so our dwrite code fakes them using the em-size for maxAdvance, and the width of 'x' for aveCharWidth.

Most of the time, the true values from the font are not really needed, but the TextControlFrame code looks at these two fields to decide if the font is "fixed-pitch" and behaves differently in response; for non-fixed-pitch fonts, some padding is added to the control width. See http://mxr.mozilla.org/mozilla-central/source/layout/forms/nsTextControlFrame.cpp#279.

Comment 4

[Jonathan Kew [:jfkthame]]

Attachment: patch, v1 - get true maxAdvance and aveCharWidth in dwrite font metrics

This resolves the problem by reading the 'hhea' and 'OS/2' tables to get the maxAdvance and aveCharWidth values, if possible (it falls back to the old "fake" values if the font tables are not available -- this shouldn't happen for valid TrueType/OpenType fonts).

Comment 5

[Bas Schouten (:bas.schouten)]

Comment on attachment 462782 [details] [diff] [review]
patch, v1 - get true maxAdvance and aveCharWidth in dwrite font metrics

One important comment, from MSDN: 'It is possible for a NULL tableContext to be returned, especially if the implementation performs direct memory mapping on the whole file. Nevertheless, always release it later, and do not use it as a test for function success.'

So we should still release it even if tableContext is NULL.

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: patch, v1.1

Updated for review comment. Also made a similar change to gfxDWriteFontEntry::ReadCMAP(), where the same issue applies. (Marking for re-review so you can check I did the appropriate thing, Bas.)

Comment 7

[Jonathan Kew [:jfkthame]]

Requesting blocking-2.0 as we need this for D2D-reftests to pass on Win7.

Comment 8

[Bas Schouten (:bas.schouten)]

Comment on attachment 462842 [details] [diff] [review]
patch, v1.1

Looks good to me!

Comment 9

[Jonathan Kew [:jfkthame]]

http://hg.mozilla.org/mozilla-central/rev/df514ee6ac84