Closed Bug 1136557 Opened 9 years ago Closed 9 years ago

Nested inline box causes unexpected offset in vertical writing modes

Categories

(Core :: Layout: Block and Inline, defect)

Product:
Core
Component:
Layout: Block and Inline
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla39
Tracking Flags:
Tracking Status
firefox39 --- fixed

People

(Reporter: xidorn, Assigned: jfkthame)

References

(Blocks 1 open bug)

Details

Attachments

(4 files, 1 obsolete file)

testcase with a bunch of nested inline box
862 bytes, text/html
Details
pt 1 - Ignore bogus ascent/descent values in 'vhea' table
1.87 KB, patch
smontagu
: review+
Details | Diff | Splinter Review
pt 2 - Round maxDescent similarly to maxAscent in nsFontMetrics
1.20 KB, patch
smontagu
: review+
Details | Diff | Splinter Review
Reftests with nested spans in vertical mode
9.99 KB, patch
smontagu
: review+
Details | Diff | Splinter Review 
It seems that each level of inline box adds a fixed offset. See the attachment. AFAICS, this happens on both Windows and Linux.
Attachment #8568990 - Attachment description: vertical-test.html → testcase with a bunch of nested inline box
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.
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.
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
I guess this bug should block bug 1099032?
Flags: needinfo?(jfkthame)
(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.
Blocks: enable-writing-mode-dev
Flags: needinfo?(jfkthame)
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.
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.
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]
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.
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
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
Attached 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.
> 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.
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: nobody → jfkthame
Status: NEW → ASSIGNED
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.
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).
Attachment #8574855 - Flags: review?(smontagu)
Attachment #8574856 - Flags: review?(smontagu)
Attachment #8575148 - Flags: review?(smontagu)
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
Closed: 9 years ago
status-firefox39: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: