Nested inline box causes unexpected offset in vertical writing modes

RESOLVED FIXED in Firefox 39

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: xidorn, Assigned: jfkthame)

Tracking

(Blocks 1 bug)

Trunk
mozilla39
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox39 fixed)

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
It seems that each level of inline box adds a fixed offset. See the attachment. AFAICS, this happens on both Windows and Linux.
(Reporter)

Updated

4 years ago
Attachment #8568990 - Attachment description: vertical-test.html → testcase with a bunch of nested inline box

Comment 1

4 years ago
Xidorn,

I notice no horizontal offset if each of the 100 opening <span> tag has a correspondent closing </span>, like in this test:

http://www.gtalbot.org/BugzillaSection/Bug1136557NestedInlinesOffset2.html

I'm not suggesting that there is no bug or that there is nothing to investigate. I believe though that your test is not a reduced, minimized test and is not an ideal test demonstrating a bug of some kind. 

When I view the source (Ctrl+U) of attachment 8568990 [details], <body> at line 13 is purple and </body> at line 26 is red; such color syntax mismatch may be suggesting that the browser isn't able to create correspondent closing </span> tag.

I am using Firefox 38.0a1 buildID=20150219021531 under Linux Kubuntu 14.10, KDE 4.14.1 64bits.
(Reporter)

Comment 2

4 years ago
It is not because you close the <span> tag, but because you actually use a different font which doesn't seem to have the same problem. If you change the locale of my testcase from "zh-CN" to nothing, you may also see there is no offset anymore.

It seems different fonts have different significance of this problem, and some fonts are not affected at all.

To make the testcase work consistently, you can set the font to Droid Sans Fallback.

Comment 3

4 years ago
Xidorn,

I see what you mean and you are correct. I've modified my test to use 20 (not 100) nested starting <span> tag with 20 closing </span> tag and I specified 'Droid Sans Fallback' and there is an horizontal offset in Firefox 38 but no horizontal offset in Chrome 40 and no horizontal offset in Prince 9.0.5.

Gérard
(Reporter)

Comment 4

4 years ago
I guess this bug should block bug 1099032?
Flags: needinfo?(jfkthame)
(Assignee)

Comment 5

4 years ago
(In reply to Xidorn Quan [:xidorn] (UTC+11) from comment #4)
> I guess this bug should block bug 1099032?

Probably. It seems to be both platform- and font-dependent, so it's a bit difficult to tell just how widespread the issue is.
Flags: needinfo?(jfkthame)
(Assignee)

Comment 6

4 years ago
It appears that the problem is related to pixel rounding of font metrics (ascent or baseline or something like that).... note that it varies wildly if you zoom in/out of the testcase, which will affect whether things round up or down at different sizes, and by how much.
(Assignee)

Comment 7

4 years ago
Oh, another thing: this doesn't occur (afaict) if text-orientation is set to sideways-right, when we use an alphabetic baseline in vertical mode; it seems to be specific to the use of the default centered baseline, in conjunction with (I suspect) certain fractional-px font metrics.
(Assignee)

Comment 8

4 years ago
It turns out this isn't specific to vertical writing modes! The same effect occurs in horizontal mode when using vertical-align:middle; e.g. http://jsfiddle.net/6777hz73/2/ (which exhibits this problem for me on Linux, OS X, and Win8, at least; results may still vary between platforms due to differences in how font metrics are calculated).

This is more likely to occur in vertical text because we use vertical-align:middle as the default when text-orientation is mixed or upright, but the bug is in fact more general.

It may be this is a regression from some of the writing-mode work; or maybe it's a bug we've had "forever". In any case, we do need to fix it.
Summary: Nested inline box causes unexpected offset in vertical writing modes → Nested inline boxes unexpectedly offset when using vertical-align:middle [was: Nested inline box causes unexpected offset in vertical writing modes]
(Reporter)

Comment 9

4 years ago
Interesting, the horizontal case shows that offset in both Chrome and IE on Windows for me, but they don't show the same problem on vertical mode when I tested it without other properties specified.
(Assignee)

Comment 10

4 years ago
Comment 8 is wrong, on closer examination. Checking the CSS spec, it says that vertical-align:middle really means

#    Align the vertical midpoint of the box with the baseline of the parent box
#    plus half the x-height of the parent.[1]

which does *not* correspond to the center-baseline alignment we want in vertical mode; and it means that the result of http://jsfiddle.net/6777hz73/2/ (in horizontal mode) is not necessarily buggy. The alignment that results is dependent on the x-height of the font being used, and the nested <span>s may accordingly be offset either upwards or downwards.

But in vertical mode using a center baseline, the code in nsLineLayout.cpp actually remaps NS_STYLE_VERTICAL_ALIGN_MIDDLE to NS_STYLE_VERTICAL_ALIGN_MIDDLE_WITH_BASELINE, which in theory should be the right thing: it should align the midpoint of the box with the baseline of the parent box, which should be its centerline, so no offset should result. I'm back to suspecting the discrepancy here is related to pixel-rounding somewhere along the way....


[1] http://www.w3.org/TR/CSS2/visudet.html#propdef-vertical-align
(Assignee)

Updated

4 years ago
Summary: Nested inline boxes unexpectedly offset when using vertical-align:middle [was: Nested inline box causes unexpected offset in vertical writing modes] → Nested inline box causes unexpected offset in vertical writing modes
(Reporter)

Comment 11

4 years ago
Posted patch a patch try to fix it (obsolete) — Splinter Review
I wrote this patch which fixes this bug, but seems to break another reftest. Maybe helpful, though.

Comment 12

4 years ago
> both Chrome and IE on
> Windows for me, (...) don't show the same problem on vertical mode when I
> tested it without other properties specified.

Chrome 40+ does not central baseline-align text in the line box when 'text-orientation' is 'mixed' (I have not checked for deprecated 'vertical-right' value).
http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/s43-baseline-inline-non-replaced-002.xht
Issue 454159: 'text-orientation: mixed' should be central baseline-aligned
https://code.google.com/p/chromium/issues/detail?id=454159

IE11.0.16 has other particularities; one is that it does not baseline-align alphabetically when text-orientation is sideways-right.
(Assignee)

Comment 14

4 years ago
It seems that the worst issues here occur with certain fonts that have a bogus 'vhea' table, with zero in all the line-spacing metrics fields. This throws off our font metrics calculations, we end up with incorrect ascent/descent values in vertical mode, and this affects the baseline alignment.
(Assignee)

Updated

4 years ago
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
(Assignee)

Comment 15

4 years ago
In addition, we have a problem in nsFontMetrics, which treats ascent and descent differently when rounding; this can result in minor (fractional-pixel) discrepancies that again affect the positioning of the vertical-centered baseline and lead to a small offset for each embedded span. Making ascent and descent have similar rounding behavior resolves this; I'm waiting for reftest results to check nothing else breaks as a result.
(Assignee)

Comment 17

4 years ago
And here are some reftests that use a range of fonts and sizes to try and make the problems here show up; they fail across all platforms with current trunk code (try run in comment 16). With the patches here, they all pass except for a small amount of residual fuzz on Windows, which seems to be a separate issue with slight inconsistencies in glyph positioning (try run in comment 13).
(Assignee)

Updated

4 years ago
Attachment #8574855 - Flags: review?(smontagu)
(Assignee)

Updated

4 years ago
Attachment #8574856 - Flags: review?(smontagu)
(Assignee)

Updated

4 years ago
Attachment #8575148 - Flags: review?(smontagu)
(Reporter)

Updated

4 years ago
Attachment #8573567 - Attachment is obsolete: true
Attachment #8574855 - Flags: review?(smontagu) → review+
Attachment #8574856 - Flags: review?(smontagu) → review+
Attachment #8575148 - Flags: review?(smontagu) → review+
https://hg.mozilla.org/mozilla-central/rev/0565095b7c08
https://hg.mozilla.org/mozilla-central/rev/f02bd650e228
https://hg.mozilla.org/mozilla-central/rev/d0a5fdddfb61
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.