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

RESOLVED FIXED in mozilla36

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jfkthame, Assigned: jfkthame)

Tracking

(Blocks: 1 bug)

unspecified
mozilla36
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
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.
(Assignee)

Comment 1

4 years ago
Created attachment 8528508 [details] [diff] [review]
Ensure conversion factor for font units is set up before trying to read vertical metrics.

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 2

4 years ago
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+
(Assignee)

Comment 3

4 years ago
(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.

https://hg.mozilla.org/integration/mozilla-inbound/rev/7b05e0d1c60e
Target Milestone: --- → mozilla37
(Assignee)

Updated

4 years ago
Target Milestone: mozilla37 → mozilla36
https://hg.mozilla.org/mozilla-central/rev/7b05e0d1c60e
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.