937268 - SVG 2 textPath/tspan getBBox method broken

Status: RESOLVED FIXED

(Core :: SVG, enhancement)

Description

[malcolm.murphy]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_6_8) AppleWebKit/534.59.8 (KHTML, like Gecko) Version/5.1.9 Safari/534.59.8

Steps to reproduce:

Hello, I'm not used to posting here so sorry if I'm not following procedure or if this is not a real bug...

Start with a simple SVG:

<svg id="svg" xmlns="http://www.w3.org/2000/svg" version="1.1" xmlns:xlink="http://www.w3.org/1999/xlink" width="800" height="400">
<defs>
<path id="myPath" d="M412.88330078125,176.73333740234375 A968.5681923990925,968.5681923990925 0 0,1 765.0479813818788,242.24454536947826" />
</defs>
<text style="fill: #000000;" font-family="'Alex Brush', sans-serif" font-size="36" font-weight="normal" font-style="normal">
<textPath id="myTextpath" xlink:href="#myPath">hello</textPath>
</text>
</svg>



Actual results:

In console: document.getElementById('myTextpath').getBBox returns "getBBox()" (indicating method is defined), however document.getElementById('myTextpath').getBBox() triggers "Method not implemented" error.

This now happens in FF 25 on both OS X and Windows 8.


Expected results:

In FF 24, document.getElementById('myTextpath').getBBox() returns an SVGRect object.

Comment 1

[Robert Longson [:longsonr]]

textPath having a bounding box is part of SVG 2. The SVG 2 specification is not finished yet.

Does this work in any other current browser?

Comment 2

[malcolm.murphy]

That's good to know about the SVG 2 spec. I just tested the latest versions of Chrome, Safari and IE 9/10 and only Chrome now returns an SVGRect object. The problem we were having is that our script tested whether getBBox existed (e.g., if (path.getBBox) then doStuffWith(path.getBBox())...), but since path.getBBox was still defined it threw it off. Easy fix in the script but seemed strange that it was taken out in a later version. Anyway thanks for your help and quick response. This issue can be closed as far as I'm concerned.

Comment 3

[Robert Longson [:longsonr]]

Not sure what you mean about later version, SVG 1.1 is the latest complete version, SVG 2 is the upcoming version, this functionality is new with SVG 2 which is why it's not implemented widely as the specification is not yet complete.

We will be implementing this at some point so we'll leave this bug open for that.

Comment 4

[malcolm.murphy]

I just meant that it worked fine in FF 24 (getBBox() returns a valid SVGRect object) but it stopped working in FF 25.

Comment 5

[Robert Longson [:longsonr]]

Good to know, perhaps that raises its priority then. 

Cam, it looks like css text frames broke this SVG 2 feature.

Comment 6

[Cameron McCormack (:heycam)]

I'll take a look (probably next week though; feel free to steal if you want, Robert).

Comment 7

[Simon A. Eugster]

Can be closed? Works now.

Comment 8

[Robert Longson [:longsonr]]

Attachment: testcase (should show alert box)

Comment 9

[Robert Longson [:longsonr]]

Doesn't work for me, which is not surprising as the code's still not been written to do this.

Comment 10

[Simon A. Eugster]

Ah. Should have read the whole bug report and not just the first half …

Comment 11

[Sebastian Zartner [:sebo]]

I've adjusted the summary to make it clearer.
Because it works for other SVG elements like SVGPathElement and was already working for <textPath> elements in previous stable versions of Firefox, I set the bug severity back to 'normal'.

(In reply to Robert Longson from comment #5)
> Cam, it looks like css text frames broke this SVG 2 feature.

I can confirm that enabling svg.text.css-frames.enabled (see bug 839955) caused the problem.

Cam, you assigned this bug to you two years ago. Are you still planning to work on this?

Sebastian

Comment 12

[Robert Longson [:longsonr]]

It was not working for textPath in any previous version.

Comment 13

[Cameron McCormack (:heycam)]

I don't have time to work on this at the moment.

Comment 14

[Sebastian Zartner [:sebo]]

(In reply to Robert Longson from comment #12)
> It was not working for textPath in any previous version.

It *was* working in Firefox 24. See comment 4. I also tried it myself earlier today and can confirm that it returned an SVGRect object for <textPath> and <tspan>. I didn't check, though, whether the values within the SVGRect were actually correct.

Sebastian

Comment 15

[Robert Longson [:longsonr]]

Pretty sure the rect would have been wrong. Would have been the rect for the complete text element and not the tspan/textPath only rect.

Comment 16

[Sebastian Zartner [:sebo]]

Attachment: Test case

(In reply to Robert Longson from comment #15)
> Pretty sure the rect would have been wrong. Would have been the rect for the
> complete text element and not the tspan/textPath only rect.

Correct. I've created a new test case, which is creates transparent blue rects indicating the bounding boxes of two <textPath> and one <tspan> based on the data returned by getBBox().
In Firefox 24 the three rects surround the whole <text> element instead. Btw. the same is happening in Chrome 47 and Opera 34, so I may file bugs for them, too.

Sebastian

Comment 17

[Sebastian Zartner [:sebo]]

(In reply to Sebastian Zartner [:sebo] from comment #16)
> In Firefox 24 the three rects surround the whole <text> element instead.
> Btw. the same is happening in Chrome 47 and Opera 34, so I may file bugs for
> them, too.

There's already an issue filed for this:
https://code.google.com/p/chromium/issues/detail?id=349835

Sebastian

Comment 19

[violet.bugreport]

Attachment: Bug 937268 - Should implement getBBox() for tspan

We implements getBBox for tspan. The implementation also has consistent
behavior as getBoundingClientRect.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ee2d5ba2854c
Should implement getBBox() for tspan r=longsonr

Comment 21

[Razvan Maries]

Backed out for perma fails on dom/svg/test/test_bbox.xhtm.

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=ee2d5ba2854cbed117403c15872de471e0bc19af&selectedJob=240204741

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=240204741&repo=autoland&lineNumber=8726

01:41:52 INFO - TEST-START | dom/svg/test/test_bbox.xhtml
01:41:52 INFO - GECKO(903) | ++DOMWINDOW == 20 (0x11dc83000) [pid = 905] [serial = 67] [outer = 0x12633dd40]
01:41:52 INFO - GECKO(903) | ++DOCSHELL 0x11c924800 == 8 [pid = 905] [id = {31bfdf88-cb29-d24d-adde-0f580edbe8ca}]
01:41:52 INFO - GECKO(903) | ++DOMWINDOW == 21 (0x11c6f43e0) [pid = 905] [serial = 68] [outer = 0x0]
01:41:52 INFO - GECKO(903) | ++DOMWINDOW == 22 (0x126c59400) [pid = 905] [serial = 69] [outer = 0x11c6f43e0]
01:41:52 INFO - TEST-INFO | started process screencapture
01:41:52 INFO - TEST-INFO | screencapture: exit 0
01:41:52 INFO - Buffered messages logged at 01:41:52
01:41:52 INFO - TEST-PASS | dom/svg/test/test_bbox.xhtml | fO.getBBox().x - got 10, expected 10 (within 0)
01:41:52 INFO - TEST-PASS | dom/svg/test/test_bbox.xhtml | fO.getBBox().y - got 10, expected 10 (within 0)
01:41:52 INFO - TEST-PASS | dom/svg/test/test_bbox.xhtml | fO.getBBox().width - got 100, expected 100 (within 0)
01:41:52 INFO - TEST-PASS | dom/svg/test/test_bbox.xhtml | fO.getBBox().height - got 100, expected 100 (within 0)
01:41:52 INFO - TEST-PASS | dom/svg/test/test_bbox.xhtml | i.getBBox().x - got 10, expected 10 (within 0)
01:41:52 INFO - TEST-PASS | dom/svg/test/test_bbox.xhtml | i.getBBox().y - got 10, expected 10 (within 0)
01:41:52 INFO - TEST-PASS | dom/svg/test/test_bbox.xhtml | i.getBBox().width - got 100, expected 100 (within 0)
01:41:52 INFO - TEST-PASS | dom/svg/test/test_bbox.xhtml | i.getBBox().height - got 100, expected 100 (within 0)
01:41:52 INFO - TEST-PASS | dom/svg/test/test_bbox.xhtml | a.getBBox().height
01:41:52 INFO - TEST-PASS | dom/svg/test/test_bbox.xhtml | a.getBBox().height
01:41:52 INFO - TEST-PASS | dom/svg/test/test_bbox.xhtml | b.getBBox().x
01:41:52 INFO - TEST-PASS | dom/svg/test/test_bbox.xhtml | b.getBBox().y
01:41:52 INFO - TEST-PASS | dom/svg/test/test_bbox.xhtml | b.getBBox().width - got 10, expected 10 (within 0.0002)
01:41:52 INFO - TEST-PASS | dom/svg/test/test_bbox.xhtml | b.getBBox().height
01:41:52 INFO - Buffered messages finished
01:41:52 INFO - TEST-UNEXPECTED-FAIL | dom/svg/test/test_bbox.xhtml | tspantext1.getBBox().x - got 19, expected 20
01:41:52 INFO - SimpleTest.is@SimpleTest/SimpleTest.js:320:16
01:41:52 INFO - compareBBox@dom/svg/test/test_bbox.xhtml:44:5
01:41:52 INFO - run@dom/svg/test/test_bbox.xhtml:65:3
01:41:52 INFO - EventListener.handleEvent*@dom/svg/test/test_bbox.xhtml:78:8
01:41:52 INFO - TEST-PASS | dom/svg/test/test_bbox.xhtml | tspantext1.getBBox().y
01:41:52 INFO - Not taking screenshot here: see the one that was previously logged

Backout: https://hg.mozilla.org/integration/autoland/rev/d6ace2daa6327706bbaad8a36ad11b8a4f721ec5

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/332599ee861c
Should implement getBBox() for tspan r=longsonr

Comment 23

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/332599ee861c