The default bug view has changed. See this FAQ.

SVGLocatable.getBBox() fails with a <text><tspan> nodes

RESOLVED FIXED in Firefox 15

Status

()

Core
SVG
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Martin Hejral, Assigned: Robert Longson)

Tracking

({regression})

14 Branch
mozilla17
x86
Mac OS X
regression
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox15+ verified, firefox16+ fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Created attachment 645860 [details]
BBox-test.svg

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.5; rv:14.0) Gecko/20100101 Firefox/14.0.1
Build ID: 20120713134347

Steps to reproduce:

//node.getBBox() call fail in FF from version 13 up to 16
var node = document.getElementById("text_tspan");
var bb = node.getBBox();
alert("BBox of <text_tspan>:   "+bb.x+", "+bb.y+", "+bb.width+", "+(bb.height));


Actual results:

Fails when called with <text><tspan> SVG DOM node construct, e.g.

<text id='text_tspan' font-size='2.80' fill='red'>
<tspan x='22' y='-1'>[22,-1]</tspan>
<tspan x='88' y='-88' text-anchor='end'>[88,-88]</tspan>
</text>

returns BBox of <text_tspan>:   0, -90.8125, 88.1875, 90.8125


Expected results:

result should be BBox of <text_tspan>:   22, -91, 66, 91
(Reporter)

Comment 1

5 years ago
Comment on attachment 645860 [details]
BBox-test.svg

.getBBox() fails with:

<text id='text_tspan' font-size='2.80' fill='red'>
<tspan x='22' y='-1'>[22,-1]</tspan>
<tspan x='88' y='-88' text-anchor='end'>[88,-88]</tspan>
</text>

...but this node returns correct BBox:

<text id='text_only' font-size='2.80' x='22' y='-88' fill='yellow'>
[22,-88]
</text>
(Reporter)

Comment 2

5 years ago
Operation getBBox() is very important for dynamic web apps. We develop huge documentation system and such bug causes fatal problems...

Bug appears until FF 13 up to 16...

Many thanks for help!!
(Assignee)

Comment 3

5 years ago
Would you be willing to find a regression range?
Keywords: regressionwindow-wanted
(Assignee)

Updated

5 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 4

5 years ago
Do do that you would follow the instructions here: https://quality.mozilla.org/docs/bugzilla/guide-to-triaging-bugs-for-firefox/finding-a-regression-window/

type about:buildconfig in the address bar in the last good build and first bad build and post the Built from link from each
(Reporter)

Comment 5

5 years ago
Pardon, right now, after additional test on Windows XP, it seems that FF13 was still OK, but FF14 already fails...
(Reporter)

Comment 6

5 years ago
(In reply to Robert Longson from comment #4)
> Do do that you would follow the instructions here:
> https://quality.mozilla.org/docs/bugzilla/guide-to-triaging-bugs-for-firefox/
> finding-a-regression-window/
> 
> type about:buildconfig in the address bar in the last good build and first
> bad build and post the Built from link from each

thx for info, I'm trying...
(Assignee)

Comment 7

5 years ago
Around February/March this year then.

Comment 8

5 years ago
(In reply to Robert Longson from comment #3)
> Would you be willing to find a regression range?
Testcase:
Before regression: BBox of <text_tspan>:   0, -90.75, 88, 90.75
After regression: BBox of <text_tspan>:   22, -90.75, 66, 90.375

m-i
good=2012-01-23
bad=2012-01-24
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e170001f3e9b&tochange=388edf50e323


It's a regression of:
Robert Longson — Bug 647914 - Horizontal and vertical SVG paths are omitted from bbox calculations if they have siblings. r=jwatt
Blocks: 647914
Keywords: regressionwindow-wanted → regression

Comment 9

5 years ago
Sorry, I mixed "Before" and "After", just switch them.
(Assignee)

Updated

5 years ago
Assignee: nobody → longsonr
(Assignee)

Updated

5 years ago
Version: 13 Branch → 14 Branch
(Assignee)

Comment 10

5 years ago
Created attachment 646083 [details] [diff] [review]
patch
Attachment #646083 - Flags: review?(jwatt)
The changes to SetInitialMatrix look like a good idea, but why are the necessary in this case? I can't see anything in the testcase that would result in a singular matrix.
(Assignee)

Comment 12

5 years ago
The matrix isn't singular. The CharacterIterator is in error before that as there's no text to work with. The problem markup is this...

<text>               <-- whitespace here 
  <tspan x= y=/>
</tspan>

The whitespace compresses to nothing but then we still try to measure the empty glyph frame giving us 0, 0, 0, 0 which then contributes to the bounding box.

The CharacterIterator class spots that there is no text and sets mInError but then there's no way to pick that up. We don't check the matrix either so this patch kills two birds with one stone.
Comment on attachment 646083 [details] [diff] [review]
patch

I see. Thanks for the explanation.
Attachment #646083 - Flags: review?(jwatt) → review+
(Assignee)

Comment 14

5 years ago
Created attachment 646104 [details] [diff] [review]
missed a gfx->restore() call in one spot
Attachment #646083 - Attachment is obsolete: true
(Assignee)

Comment 15

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c0a0c727388
Flags: in-testsuite+
Target Milestone: --- → mozilla17
https://hg.mozilla.org/mozilla-central/rev/7c0a0c727388
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 17

5 years ago
Martin,

The SVG team could really do with some help catching problems like this _before_ they reach the release builds. We don't seem to have enough people testing SVG before that stage right now. The Beta builds are very stable (get virtually no changes before release), so if you'd be willing to use Beta or Aurora builds for your day-to-day browsing and report any regressions that you notice, that would be a great help. If you're interested, you can find Beta/Aurora builds here:

http://www.mozilla.org/en-US/firefox/channel/
(Assignee)

Comment 18

5 years ago
Comment on attachment 646104 [details] [diff] [review]
missed a gfx->restore() call in one spot

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 647914
User impact if declined: Wrong bounding boxes for text elements that have children with whitespace between the children. Can make selection difficult.
Testing completed (on m-c, etc.): landed on m-c with test
Risk to taking this patch (and alternatives if risky): Low risk, no realistic alternatives other than not taking the patch though
String or UUID changes made by this patch: none
Attachment #646104 - Flags: approval-mozilla-beta?
Attachment #646104 - Flags: approval-mozilla-aurora?
Because this is already broken since 13 and we're not getting a ton of complaints I'd like to have a better sense of how widespread this issue is and what kind of user pain it might be causing, I'm inclined to let it ride the trains without a persuasive case here.
(Reporter)

Comment 20

5 years ago
(In reply to Lukas Blakk [:lsblakk] from comment #19)
> Because this is already broken since 13 and we're not getting a ton of
> complaints I'd like to have a better sense of how widespread this issue is
> and what kind of user pain it might be causing

Lukas,

we develop web app for corporate environment which works online with huge documents converted from AutoCAD and we need to interactively work with complex vector structures like circuit diagrams etc. 
 
It is essential to have possibility to compute sizes of objects in different Coordinate Systems, for example if you need to add some new objects to the complex SVG structure...

This is exactly the case for which SVG is intended!

The fact, that there are almost no such web application at this time — this fact is no reason to block development of such apps ;-) 

Also there is big potential for such applications in various large organizations...
(Assignee)

Comment 21

5 years ago
Have you checked out a nightly to confirm that you're happy this issue is fixed there Martin?
(Reporter)

Comment 22

5 years ago
(In reply to Robert Longson from comment #21)
> Have you checked out a nightly to confirm that you're happy this issue is fixed there Martin?

Nice work Robert: My tests are OK

FF13 OK
FF14 broken
...
FF17.0a1 (2012-08-09) OK

Many thanks to everybody for great work. 

to comment #17: we will try to use beta more offen...

Robert, please one more stupid question :) This fix will appear in FF17? Is this how Mozilla development works?
(Assignee)

Comment 23

5 years ago
By default, yes. If approved for aurora it jumps up one step to 16 and if approved for beta it would jump 2 steps to 15.
Comment on attachment 646104 [details] [diff] [review]
missed a gfx->restore() call in one spot

Approving for branches since this only touches svg code, fixes a regression, and we can easily verify on 15/16 that this is resolved.  Please report back with your results once Beta 5 ships next Friday (Aug 18)
Attachment #646104 - Flags: approval-mozilla-beta?
Attachment #646104 - Flags: approval-mozilla-beta+
Attachment #646104 - Flags: approval-mozilla-aurora?
Attachment #646104 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 25

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/719ca729259e
status-firefox16: --- → fixed
tracking-firefox15: --- → ?
tracking-firefox16: --- → ?
(Assignee)

Comment 26

5 years ago
https://hg.mozilla.org/releases/mozilla-beta/rev/3ee4b5b63f68
status-firefox15: --- → fixed
tracking-firefox15: ? → +
tracking-firefox16: ? → +
(In reply to Martin Hejral from comment #0)
> Expected results:
> 
> result should be BBox of <text_tspan>:   22, -91, 66, 91

Actual results loading the test case on Nightly 17.0a1 (2012-08-14):
BBox of <text_tspan>: 21.875, -90.75, 66.25, 90.375

Is this correct? And what about the second pop-up with BBox of <text_only>? It is not important?
(Assignee)

Comment 28

5 years ago
You'll get slightly different results on different platforms which is OK. If the numbers are wildly out e.g. 0, -90.75, 88, 90.75 then that's bad. 

I don't think the second dialogue is relevant, that case works.
(Reporter)

Comment 29

5 years ago
(In reply to Paul Silaghi [QA] from comment #27)
> (In reply to Martin Hejral from comment #0)
> > Expected results:
> > 
> > result should be BBox of <text_tspan>:   22, -91, 66, 91
> 
> Actual results loading the test case on Nightly 17.0a1 (2012-08-14):
> BBox of <text_tspan>: 21.875, -90.75, 66.25, 90.375
> 
> Is this correct? 

YES, correct. By my opinion this effect is caused by different engines for font computing/rendering (fonts usually have complex vector definitions...)

> And what about the second pop-up with BBox of <text_only>?
> It is not important?

The second pop-up was added in testcase to demonstrate fact that bug will appear only when <text> contains <tspan> elements... not for simple <text> like this:

<text id='text_only' font-size='2.80' x='22' y='-88' fill='yellow'>
[22,-88]
</text>
Keywords: verifyme
Actual results loading the test case on FF 15b5:
on Mac OS X 10.6: BBox of <text_tspan>: 21.875, -90.75, 66.25, 90.375
on Win 7/64: BBox of <text_tspan>:   22, -90.75, 66, 90.375

Verified fixed based on comment 28 and comment 29.
Able to see the issue on FF 15b4.
status-firefox15: fixed → verified
Keywords: verifyme
QA Contact: paul.silaghi
You need to log in before you can comment on or make changes to this bug.