Closed
Bug 1082017
Opened 10 years ago
Closed 10 years ago
writing-mode:vertical-* affects computed value of line-height, leading to mochitest failures
Categories
(Core :: Layout: Text and Fonts, defect)
Core
Layout: Text and Fonts
Tracking
()
RESOLVED
FIXED
mozilla38
Tracking | Status | |
---|---|---|
firefox38 | --- | fixed |
People
(Reporter: jfkthame, Assigned: jfkthame)
References
(Blocks 1 open bug)
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
dbaron
:
review+
|
Details | Diff | Splinter Review |
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•10 years ago
|
Blocks: writing-mode
Assignee | ||
Comment 1•10 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.)
Comment 2•10 years ago
|
||
(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?
Updated•10 years ago
|
Assignee | ||
Updated•10 years ago
|
Blocks: enable-writing-mode-dev
Assignee | ||
Comment 3•10 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•10 years ago
|
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Updated•10 years ago
|
Attachment #8562251 -
Flags: review?(dbaron) → review+
Assignee | ||
Comment 4•10 years ago
|
||
Target Milestone: --- → mozilla38
Comment 5•10 years ago
|
||
sorry had to back this out in https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=e064c1c87d02 since one of this changes caused https://treeherder.mozilla.org/logviewer.html#?job_id=6509981&repo=mozilla-inbound
Flags: needinfo?(jfkthame)
Assignee | ||
Comment 6•10 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•10 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•10 years ago
|
Attachment #8562251 -
Attachment is obsolete: true
Updated•10 years ago
|
Attachment #8565435 -
Flags: review?(dbaron) → review+
Assignee | ||
Comment 8•10 years ago
|
||
Comment 9•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox38:
--- → fixed
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•