1142982 - Value "middle" of SVG attribute dominant-baseline is treated as "central"

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Oleg Torbasow]

Attachment: middle.svg

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:36.0) Gecko/20100101 Firefox/36.0
Build ID: 20150305021524

Steps to reproduce:

Set SVG attribute dominant-baseline to "middle".


Actual results:

Text position as dominant-baseline has value "central", the distance between middle baseline and alphabetic baseline equals to 0.5em.


Expected results:

Value middle "identifies a baseline that is offset from the alphabetic baseline in the shift-direction by 1/2 the value of the x-height font characteristic". So the distance between middle baseline and alphabetic baseline must be equal to 0.5ex.
Google Chrome 38 does so, but not the Firefox.

Comment 1

[Cameron McCormack (:heycam)]

We should be able to handle mapping middle internally to a vertical-align:0.5ex value.  ConvertSVGDominantBaselineToVerticalAlign in nsFrame.cpp will need to return an nsStyleCoord rather than a uint8_t.

Comment 2

[Cameron McCormack (:heycam)]

Actually, we already map dominant-baseline:middle to vertical-align:middle, which does shift things by 0.5ex (see where NS_STYLE_VERTICAL_ALIGN_MIDDLE is mentioned in nsLineLayout::VerticalAlignFrames) -- I wonder why isn't that enough?

Comment 3

[Cameron McCormack (:heycam)]

Oh, I think it is GetBaselinePosition in SVGTextFrame.cpp that needs updating.

Comment 4

[Robert Longson [:longsonr]]

Attachment: baseline.txt

Like this?

Comment 5

[Cameron McCormack (:heycam)]

Comment on attachment 8588653 [details] [diff] [review]
baseline.txt

Review of attachment 8588653 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!  Please add a test too, something along the lines of comparing <text dominant-baseline="middle">abc</text> to <text dy="0.5ex">abc</text>.

Comment 6

[Robert Longson [:longsonr]]

For some reason it just doesn't work quite right. Presumably the units conversion is wrong somehow.

Comment 7

[Robert Longson [:longsonr]]

Attachment: baseline.txt

worth another review since I clearly didn't get the units right last time.

Comment 8

[Robert Longson [:longsonr]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e0ea57133f02

Comment 9

[Robert Longson [:longsonr]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e0ea57133f02

Comment 10

[Robert Longson [:longsonr]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e91b500ca39c

Comment 11

[Robert Longson [:longsonr]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=375abe9704ee

Comment 12

[Robert Longson [:longsonr]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/efa8f733c5a4

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/efa8f733c5a4