1109860 - SVG `getBBox` dimensions include hidden characters pertaining to bidirectional text

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Matt Wistrand]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:36.0) Gecko/20100101 Firefox/36.0
Build ID: 20141210004006

Steps to reproduce:

1. Generate string with left-to-right mark (\u200E, although other characters such as \u202A cause the same problem) prepended.
2. Add aforementioned string to an SVG text node.
3. Call text node's `getBBox` method.


Actual results:

Different `x`, `y`, `height`, and `width` values are returned than the same node without the prepended bidirectional mark(s). A working example can be found at http://jsfiddle.net/mwistrand/aph95n4s/


Expected results:

The `x`, `y`, `height`, and `width` values returned by `getBBox` should be calculated as if the bidirectional mark(s) were not present.

Comment 1

[Loic]

Regression range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e85b0372cece&tochange=3ad5ecf34c9e

Robert Longson — Bug 919900 - Correct bounding box calculations for empty text elements with svg.text.css-frames.enabled. r=heycam

Comment 2

[Robert Longson [:longsonr]]

Did we really get it right before bug 919900 or did we get both cases identically wrong at that time?

Comment 3

[Robert Longson [:longsonr]]

Attachment: lrm.txt

Comment 4

[Robert Longson [:longsonr]]

FWIW the !r.IsEmpty() is the fix (length == 0 is an optimisation)

Comment 5

[Loic]

(In reply to Robert Longson from comment #2)
> Did we really get it right before bug 919900 or did we get both cases
> identically wrong at that time?

Yes, it was right before bug 919900, same values in both cases (from the reporter's testcase).

Comment 6

[Loic]

Before bug 919900 in both cases:
    x: 30
    y: 16.399999618530273
    width: 166.61666870117188
    height: 17.600000381469727

Comment 7

[Cameron McCormack (:heycam)]

Should we instead be checking !fill.IsEmpty()?  Otherwise, if we have some real text and aFlags == eIncludeStroke, we'll end up returning an empty bbox rather than the bbox that just takes into account the stroke.

Comment 8

[Robert Longson [:longsonr]]

Attachment: address review comment

Comment 9

[Robert Longson [:longsonr]]

https://tbpl.mozilla.org/?tree=Try&rev=babb3e805a9e

Comment 10

[Robert Longson [:longsonr]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/35141bed8113

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/35141bed8113