writing-mode:vertical-* affects computed value of line-height, leading to mochitest failures

RESOLVED FIXED in Firefox 38

Status

()

defect
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: jfkthame, Assigned: jfkthame)

Tracking

(Blocks 1 bug)

unspecified
mozilla38
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox38 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Assignee

Description

5 years ago
With current trunk code, enabling support for vertical writing modes (the compile-time option in WritingModes.h, and the runtime vertical-text pref) will lead to a number of failures in layout/style/test/test_inherit_computation.html; see https://tbpl.mozilla.org/?tree=Try&rev=cbf75f7e0bc8.

The issue here is that changing the writing mode to vertical will affect the computed value of 'normal' line-height, because we'll be using a different set of font metrics (see bug 1065002) as the source of ascent/descent/linegap values. This comes as a surprise to test_inherit_computation.html, which doesn't expect changes to "unrelated" properties to affect line-height (or the font shorthand, which includes the line-height value).

We need to consider whether it's OK for writing-mode to affect normal line-height in this way, or if it should remain constant across writing-mode changes. (The "best" answer here is not entirely clear to me; it seems to depend on the exact vertical-text use case one has in mind, and could also depend on the value of text-orientation. But then, do we want text-orientation to affect line-height...?)

If we conclude such variation *is* legitimate, we'll need to adjust test_inherit_computation.html so that it allows for it; or if not, we need to fix the line-spacing code to provide writing-mode-independent results.
Assignee

Updated

5 years ago
Blocks: writing-mode
Assignee

Comment 1

5 years ago
Here's a simple testcase that displays the issue here, whereby line-height reports different values when writing-mode is changed. This exhibits the issue on OS X; behavior on other platforms may vary due to font availability, and because standard (horizontal) font metrics are provided by platform-specific code that has some inconsistencies at present.

(Note that this also illustrates a couple of other issues we currently have: changing text-orientation does not affect the reported value of line-height, but does in fact change the rendered line spacing; and enabling vertical writing mode for the test <div> causes a vertical scrollbar to appear on the window, which shouldn't be needed. But these are separate issues from the question of how writing-mode and line-height *ought* to interact.)
(In reply to Jonathan Kew (:jfkthame) from comment #0)
> If we conclude such variation *is* legitimate, we'll need to adjust
> test_inherit_computation.html so that it allows for it; or if not, we need
> to fix the line-spacing code to provide writing-mode-independent results.

Or maybe just add to the prerequisites line for line-height in property_database.js?
Blocks: 1079125
No longer blocks: writing-mode
Assignee

Comment 3

4 years ago
This prevents a bunch of errors from test_inherit_computation.html when writing-mode:vertical-* is enabled, due to font metrics being different depending on the orientation.
Attachment #8562251 - Flags: review?(dbaron)
Assignee

Updated

4 years ago
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Attachment #8562251 - Flags: review?(dbaron) → review+
Assignee

Comment 6

4 years ago
Drat, this can't land until we enable writing-mode (in bug 1099032), as it'll cause mochitest errors when the writing-mode property is not available.
Flags: needinfo?(jfkthame)
Assignee

Comment 7

4 years ago
Here's an alternative version of the patch, moving the addition of the prerequisite inside of the test for whether writing-mode is enabled; this should allow us to land this patch independently of when we actually toggle the pref.
Attachment #8565435 - Flags: review?(dbaron)
Assignee

Updated

4 years ago
Attachment #8562251 - Attachment is obsolete: true
Attachment #8565435 - Flags: review?(dbaron) → review+
https://hg.mozilla.org/mozilla-central/rev/f20cf9991555
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.