New issue
Advanced search Search tips

Issue 735505 link

Starred by 3 users

Issue metadata

Status: WontFix
Owner: ----
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-02-21
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

RenderText should vertically center the cap height for multi-line text

Project Member Reported by jdonnelly@chromium.org, Jun 21 2017

Issue description

It does this for non-multi-line text with DetermineBaselineCenteringText:
https://cs.chromium.org/chromium/src/ui/gfx/render_text.cc?l=1326&rcl=5e77d8045bc6904a6925dced1187f3e7dd8d5ae2

But for multi-line text, the same adjustment isn't made, based on the logic here:
https://cs.chromium.org/chromium/src/ui/gfx/render_text.cc?l=1205&rcl=5e77d8045bc6904a6925dced1187f3e7dd8d5ae2

As a result, multi-line text tends to appear as if it's laying out too low compared to non-multi-line text. This specifically causes an issue in the new experimental vertical layout in the omnibox. Compare the vertical position of the "Google Search" text (non-multi-line) and the "characterized by" text (multi-line) in the attached screenshots.

The following is a bit of a hack but it shows how it's possible to center the cap height of multi-line text: https://codereview.chromium.org/2948033003.

Are there any drawbacks or implications of doing something like this? Is there a better approach to achieving the desired effect?
 
omnibox1.PNG
24.1 KB View Download
omnibox2.PNG
23.5 KB View Download

Comment 1 by tapted@chromium.org, Jun 22 2017

I'm not sure we should be adding more logic to RenderText::GetAlignmentOffset(). I don't think it's used in RenderTextHarfBuzz::EnsureLayout(), and layout gets complicated when the logic drifts between the two.

Is there a fix that involves doing something like adding a `line` argument to GetDisplayTextBaseline() so that the logic in GetAlignmentOffset() can be simplified?

(note: I started using DetermineBaselineCenteringText() for multiline layout in https://codereview.chromium.org/2767163003 - maybe that needs a tweak so that the baseline reported by a line-number-aware GetDisplayTextBaseline() returns what you want)
Is there a better component than the generic "UI" component for this bug so that it doesn't get lost entirely?

Components: -UI UI>Browser>Core
jdonnelly@, if this matters to you, can you pursue finding an appropriate owner?  Otherwise please close it.
NextAction: 2018-02-21
ping jdonnelly@ per comment #3
The NextAction date has arrived: 2018-02-21
Status: WontFix (was: Untriaged)
We've abandoned vertical layout, this is obsolete.

Sign in to add a comment