188467 - incorrect position of certain accents

Status: RESOLVED FIXED

(Core :: MathML, defect)

Description

[rbs]

When fixing-up the height of accents to cater for fonts with weird metrics in
<mo>accent</mo>, the inner child text frame wasn't adjusted to compensate the
effect. Will attach a simple patch that leaves the ascent of the <mo> container
frame unchanged. With this, it is not necessary to do the adjustements since, in
this case, the descent doesn't impact the coordinates of the inner text frame.

Comment 1

[rbs]

Attachment: testcase

Comment 2

[rbs]

Attachment: patch

bz, r+sr? (No need to take too much of your time trying to analyze/understand
the whole stuff in great details. It is too MathML specific.)

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 111140 [details] [diff] [review]
patch

Add a nice comment explaining why you set one but not the other, ok?

Comment 4

[rbs]

Attachment: alternate patch - more agressive

Actually, I am going to comment a bit more in this bug (with a nice screenshot
:-), and simply refer to the bug. First, I have this alternate patch to get
some
feedback.

Comment 5

[rbs]

Attachment: screenshot - rendering before and after the patches

Since MathML frames overlap a lot, the whole point of these genuflections is to
tweak a nice "height" for the accent frame so that it plays nice with other
things such as a background or a caret (e.g., in the context of Steve's JS
editor). Just using the default font ascent looks weird, and even more so in
the face of fonts with weird metrics (common with math fonts). Hence, painting
the background of the accent will protrude and overlap the base for example.
Because the tweaking of the size is happening after the inner text frame has
been positioned, adjustements are needed to sync its coordinates (since these
are relative to its parent). Due to an oversight, I was not doing that in the
code. In patch v.1, I simply removed the mangling that was done to container's
ascent knowing that it would then remove the need to adjust the coordinates of
the child. As you can see in the screenshot, the top of the backgroung of the
accent is left big. In the more aggressive path v.2, I re-instated the earlier
intention and fixed the oversight fully. So v.2 is cutting off the excess on
the top too, as the screenshot shows. The domino effect is to propagate the
delta to the child so that its (relative) coordinates achieve the desired
geometrical effect -- and that is what the patch is doing (note: |isAccent| can
only be true with a single child).

Comment 6

[rbs]

Attachment: testcase used for the screenshot

It is the same testcase as attachment 111139 [details], but to show the computed
background area of those elements <mo accent="true">...</mo>, I simply added
this CSS rule:
<style>
[accent="true"] { background-color: cyan; }
</style>

Comment 7

[rbs]

Fixed.

Comment 8

[steve.swanson]

I was happy with the first patch (since I don't want the caret to appear on 
the accent anyway).  But the second patch seems to be better, giving more 
accurate heights for the accent frames.

Comment 9

[rbs]

-> really marking FIXED.