Closed Bug 1102406 Opened 8 years ago Closed 8 years ago

incorrectly-sized (tiny) caret in vertical text, depending on font/size/platform


(Core :: Layout: Text and Fonts, defect)

Not set





(Reporter: jfkthame, Assigned: jfkthame)


(Blocks 1 open bug)



(1 file)

In some cases, I'm seeing a tiny caret (just a couple of px wide, by the look of it) in editable text elements with vertical writing mode (<input>, <textarea>, or general HTML content with the contenteditable attribute).

This seems to depend on the font and size in use; presumably we're getting bad metrics in some cases.

E.g. in testing on WinXP, I get the correct caret in

  data:text/html,<div style="font:16px times new roman;
                  writing-mode:vertical-rl" contenteditable>foo bar

but if I change the size of the font, OR change it from Times to Arial, the caret becomes a tiny (hard-to-spot) mark at roughly the ex-height of the text.

I've also seen this on Linux; but have not (yet?) seen it happen on OS X.
The problem here occurs because on some font back-ends, mFUnitsConvFactor may not have been set up yet by the time we want to call gfxFont::CreateVerticalMetrics(). (It depends whether we've asked for horizontal metrics from the font already.) So we need to handle that case before we go multiplying values from the font tables by zero, and getting useless results.
Assignee: nobody → jfkthame
Attachment #8528508 - Flags: review?(jdaggett)
Comment on attachment 8528508 [details] [diff] [review]
Ensure conversion factor for font units is set up before trying to read vertical metrics.

Review of attachment 8528508 [details] [diff] [review]:

r+ with use of gfxFontEntry::UnitsPerEm()

::: gfx/thebes/gfxFont.cpp
@@ +3355,1 @@
>      const uint32_t kHheaTableTag = TRUETYPE_TAG('h','h','e','a');

If UnitsPerEm() is used, this can be omitted.

@@ +3381,5 @@
> +            uint16_t upem = head->unitsPerEm;
> +            if (upem >= 16 && upem <= 16384) {
> +                mFUnitsConvFactor = GetAdjustedSize() / upem;
> +            }
> +        }

It seems to me that we should be using gfxFontEntry::UnitsPerEm() here rather than duping the same code.
Attachment #8528508 - Flags: review?(jdaggett) → review+
(In reply to John Daggett (:jtd) from comment #2)
> It seems to me that we should be using gfxFontEntry::UnitsPerEm() here
> rather than duping the same code.

Indeed we should -- I forgot we had that.
Target Milestone: --- → mozilla37
Target Milestone: mozilla37 → mozilla36
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.