Closed Bug 612118 Opened 14 years ago Closed 4 years ago

SVGLocatable.getBBox() fails unless the SVG element it is applied to is attached and rendered

Categories

(Core :: SVG, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla77
Webcompat Priority P3
Tracking Status
firefox77 --- fixed

People

(Reporter: laaglu, Assigned: emilio)

References

Details

(Whiteboard: [webcompat], [wptsync upstream])

Attachments

(3 files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.11) Gecko/20101028 Mandriva Linux/1.9.2.11-0.2mdv2010.1 (2010.1) Firefox/3.6.11
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.11) Gecko/20101028 Mandriva Linux/1.9.2.11-0.2mdv2010.1 (2010.1) Firefox/3.6.11

Hello,

I am having problems with the way firefox manages calls to SVGLocatable.getBBox(). It seems the call will only succeed if it belongs to an SVG element which has already been attached and rendered. Otherwise, the call fails with a message like this one in the JavaScript console:

Error: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDOMSVGLocatable.getBBox]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: file:///home/laaglu/workspaces/lab/svglab/pbFirefoxBBox.html :: earlyBbox :: line 34"  data: no]

This is annoying for several reasons:
1/ other browsers (Opera, Chrome, Safari) do not seem to have this restriction, so this hurts portability of web applications.
2/ From a logical point of view, one does not see why the call should fail: after all, the geometry of the object has been defined, all the data to compute the bounding box exists.
3/ In some cases it is impossible to get the bounding box of the object at all. I ran into the a case where I would like to have the bounding box of a clipped shape. I would like to compute it by computing the intersection of the bbox of the shape and the intersection of the bbox of the clipPath. Except I can never get the bbox of the clipPath as it is never truly rendered.



Reproducible: Always

Steps to Reproduce:
I have attached a small sample to demonstrate the issue. Here is how it works:
a/ open the attached file (pbFirefoxBBox.html)
b/ Click the "lateBbox" button. A shape is created and attached. The subsequent call to getBbox succeeds.
c/ Click the "earlyBbox" button. A shape is created but not yet attached. The subsequent call to getBbox fails.
d/ Click the defsBbox" button. A shape def is created, a use for this shape is created and both are attached. The subsequent call to getBbox fails on the shape.

Actual Results:  
Step c fails and generates a javascript error in the console
Step d partially fails and generates a javascript error in the console


Expected Results:  
Step c should display a yellow ellipse and display its bbox in an alert window
Step d should display a green ellipse and display its bbox in an alert



The problem exists with FF3.x and FF4b7 on linux and also on FF3.x win32 (not tested FF4b7 win32)
> the geometry of the object has been defined, all the data to compute
> the bounding box exists.

That's not true if the node is not in a document; the geometry depends on the parents, in general, no?
My understanding, though I am not sure I interpret correctly, it that the bbox is expressed in the coordinate system of the shape (user space), so it does not depend on whatever transform the parent defines. The W3C doc in the IDL says:

getBBox() : "Returns the tight bounding box in current user space (i.e., after application of the transform attribute, if any) on the geometry of all contained graphics elements, exclusive of stroking, clipping, masking and filter effects). Note that getBBox must return the actual bounding box at the time the method was called, even in  case the element has not yet been rendered."
It doesn't depend on the parent's transform, but it can depend on the parent's style (e.g. font sizes, etc).

And we don't even have a concept of font size for elements that are not in a document, since it depends on the exact document the element would be in, the entire parent chain, and so forth.

It's possible to return a bounding box without reference to parents in an SVG renderer that doesn't also support CSS, but if you support both then the only way to do it is to make up random values for CSS properties.
I understand your point regarding CSS and SVG. However I am still not convinced in the following areas:

1/ I think FF should not throw an exception: the W3C spec for SVGLocatable.getBBox() does not declare exceptions (no "raises(SVGException)" in the IDL), so it is supposed to always be able to return a value. If the value cannot be computed (because the dimensions are defined in percentage units for example), I think it should return (0,0,0,0), not raise an exception. This is what Webkit and Opera do for my second example (earlyBbox).

2/ In my third example, the defsBbox, the element is attached to the DOM tree, but FF still is not able to return a value for the bbox (contrary to Webkit and Opera). I do not understand why.

3/ Would it not be feasible to return a bbox for cases where there is no ambiguity introduced by CSS. If one uses absolute pixel units, as for example in <ellipse cx="100" cy="100" rx="20" ry="10"/>, won't the bbox always be (x=80,y=90,w=40,h=20) ?
> 1/ I think FF should not throw an exception:

That's why the bug is still open, right?  ;)

> I do not understand why.

The current getBBox() implementation uses the rendering model to generate the bbox.  This is, generally, needed for things like text, which actually have to be rendered to figure out their bounds.

Gecko doesn't generate a rendering model for subtrees of display:none elements (like <defs>), because in CSS there is never a need for rendering of such subtrees.  This is not quite the case in SVG; there are existing bugs on this.  But the upshot is that getBBox has nothing to operate on in this case.  It could, of course, simply return (0,0,0,0) in all such situations; I'm not sure that would be much better than clearly reporting that it can't compute a bbox.

> Would it not be feasible to return a bbox for cases where there is no
> ambiguity introduced by CSS.

And where there is no text involved, etc.  Yes, if you wrote a completely separate codepath for reporting bboxes.... this codepath will then tend to get out of sync with the rendering-based codepath, both in terms of what it's trying to do and in terms of just computing things slightly differently for anything but the simplest cases...

It could be done, with a lot of complexity, and the bounding boxes wouldn't be all that reliable for things more complicated than a single shape, I suspect.  But yes, this is the other reason the bug is still open.
Ok I have run out of ammunitions ;-) Thanks for all your explanations. I let you decide whatever you think is best/feasible for your product.
I'm running into this too, but I'm not sure why. The object I'm trying to getBBox on seems to be rendered and visible (it is inserted into the SVG tree by means of an Ajax call).  I understand the arguments for not fixing it above, but are there workarounds?  Why should this fail on a visible object?
> The object I'm trying to getBBox on seems to be rendered and visible

In which case you're not seeing this bug, right?

Past that, it's hard to say anything without seeing your code...  Please file a separate bug on your problem with more information?
I don't see why FF couldn't add the element to the tree itself, get the bounds and then remove it again.
It's costly, but better than having the developer do this.
Add it to the tree _where_?  The bounding box of an element in the tree depends on its ancestors, in general.  Of course that was mentioned earlier in the bug; you did read the bug, right?
(In reply to Boris Zbarsky (:bz) from comment #13)
> Add it to the tree _where_?  The bounding box of an element in the tree
> depends on its ancestors, in general.  Of course that was mentioned earlier
> in the bug; you did read the bug, right?

the spec says:
Returns the tight bounding box in current user space (i.e., after application of the ‘transform’ attribute, if any) on the geometry of all contained graphics elements, exclusive of stroking, clipping, masking and filter effects). Note that getBBox must return the actual bounding box at the time the method was called, even in case the element has not yet been rendered.

So, in this case, there is no transform attribute on the ancestors (since they don't exist) but there might be one on the element itself.
So, it sound like you should act as if the element was directly on the SVG root to determine the bounds.
FWIW, I think that transform should not be included when calculating the bbox...
The issue isn't transform attributes.  It's basic things like "the bounding box of text depends on the font-size, which depends on the CSS styles".
so, what if there is no style?
I agree that it might be odd to ask for the bbox that has CSS styling, but that would really be an error for the user if he thinks that the bounds are the same regardless of styling.
> so, what if there is no style?

Then the spec doesn't actually happen to define the behavior.
I don't see that in the spec.
Anyway, it should be cleared up (and I have a vague recollection that we're already doing that)
> I don't see that in the spec.

Yes, precisely.  It simply doesn't cover the case when no style data is available; it assumes such data is always available.
Yup, the SVG WG is aware of this issue, and once they clarify what the behavior should be, then we'll make getBBox do that. I agree that in many cases getBBox could still return a useful result.
OS: Linux → All
Hardware: x86 → All
Because I am lost in the details of this issue, I will appreciate a comment on whether the problem I am struggling with is the same or related. In my case, everything is simple, the element has been rendered, there is no ambiguity about the meaning of a bounding box, and the style data is present. Here's what happens:

1. I render a text node on an SVG canvas
2. I call getBBox() on it, and it returns the correct value
3. I set the display style on the SVG node to 'none'.
4. Calling getBBox() on the same text node while its parent is not visible results in:

Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDOMSVGLocatable.getBBox]

This seems unfair, regardless of whether or not it's in the spec, and it works fine in all other browsers. I wonder whether I should think of ways to work around this bug or is there a chance it will get fixed sooner?
You're seeing exactly this bug, and the problem is that once display goes to "none" the exact rendering (and hence the exact bounding box) is no longer quite defined by the specs....
Thank you Boris. I found a solution once I started looking: Instead of display, I can use visibility to turn the layer on and off, and doing so preserves the layout while visibility is set to 'hidden', so I get the correct bbox.

Setting the display style to none/block is the default method of changing visibility in YUI, and guess it makes a better default generally, but I do not need it in the app I am building. My layers are positioned absolutely, so hiding them does not require changes in the layout.
For anyone else getting bitten by this bug and needs to ship code today I found that changing over from .getBBox() to .getBoundingClientRect() was an easy work around. It seemed to work well in all browsers I tested for my site: http://www.capitalcharts.com

.getBoundingClientRect() just returns 0 for all its properties when an element is display:none.

In case anyone else out there comes across this bug are there issues that should be known before someone converts all their .getBBox() code over to .getBoundingClientRect()?
getBoundingClientRect will return different answers if the content or its ancestors are transformed.
I also run into this problem. I tried to reference a shape that I _don't_ want to get displayed but still need the bounding box. I attached a simple example.

The rect is in the defs section. So it has a parent, there is no excuse like CSS inheritance or anything else. In this case the rect should have a bounding box.

<defs>
  <rect x="0" y="0" width="600" height="200" id="e" />
</defs>

getBBox however gives the error as mentioned before, and getBoundingClientRect just returns a rect with no dimension.

Bz, you compared the defs section with specifying display:none. The reason why no render object is created. I would disagree with that.

In webkit the <defs> element has it's own renderer that allows to calculate the layout, but avoids the actual painting. This way, we can still provide all layout information of all decedent elements. Because all decedent elements have a renderer, but just don't get painted.
Hmm.  At one point we actually had <defs> styled with display:none, I think, but that's certainly not the case now.  We certainly have a render object for both the <defs> and the <rect> in the testcase in comment 26.

It looks like we explicitly turn off getBBox for this case for some reason (via NS_STATE_SVG_NONDISPLAY_CHILD).  It's probably worth filing a separate bug for the <defs> issue, since it's completely separate from the "display: none" and "not in the DOM" issues at this point.
Could just unduplicate bug 703863.
Here is a JS Fiddle live example illustrating the issue. It's best to run this with the console open to see the DOM exception.

http://jsfiddle.net/sym3tri/kWWDK/
Here is another JSFiddle, which shows that the problem occurs even if the display:none is outside the SVG element (i.e. if the <svg> element is inside a <div> with display:none).

http://jsfiddle.net/qZYrK/
Status: UNCONFIRMED → NEW
Ever confirmed: true
I encountered this problem today. For me the error happens because of display: none or visibility: hidden on the svg element which is a necessary feature.

I had implemented a hack using setTimeout(function_reference, timeoutMillis); after display is set to NOT none and this seems to work for now.
You are mistaken, this does not happen with visibility:hidden, only display:none.
I came across this today, and I propose that the method just return undefined for unrendered nodes.  I appreciate the issues associated with style application and parent transformations, etc. and it seems to me that undefined is appropriate in this case. It is certainly an intuitive reflection of the node's state, and doesn't require just punting to a (0,0,0,0) value (however useful that is).

It would also allow developers to handle any errors resulting from expected dimensions in their own code, rather than have to use a try/catch block just to wrap a call to a native method which only queries the properties of a valid, existing node.
Ran into this today. Can we at least get a nice error message? In firefox 37 all I see is "NS_ERROR_FAILURE: "
I ran into this today as well, and I agree with Jonathan Gala that returning undefined here would be a simple and good solution.
(In reply to uberjason from comment #36)
> I ran into this today as well, and I agree with Jonathan Gala that returning
> undefined here would be a simple and good solution.

I'm undecided as to whether that would be an improvement over the current situation.  It might be a good stop-gap, or it might make no practical difference.  However, whichever the case, it certainly wouldn't be sufficient to actually resolve this bug.

If I create a rectangle in an <svg> element with x = 0, y = 0, width = 20, height = 20 and I call getBBox() on that element, I would expect the browser to return x = 0, y = 0, width = 20, height = 20, regardless of whether it is currently visible.  I mean, why not?  This isn't quantum physics!  We can know the state of our SVG without having to observe it!

If HTMLImageElement.width was throwing an error when you tried to access it for a non-displayed image this would have been fixed immediately, and fixed to return the correct image size, not 'undefined'.  I don't really see how this is any different.

There may be technical reasons why this can't be done "right now" but the aim of this bug is to fix the issue, not sweep it under the carpet.  It makes absolutely no logical sense at all to claim that returning 'undefined' (or anything other than the correct dimensions) is either correct, appropriate or desirable.
(In reply to Mark Clements from comment #37)
> If I create a rectangle in an <svg> element with x = 0, y = 0, width = 20,
> height = 20 and I call getBBox() on that element, I would expect the browser
> to return x = 0, y = 0, width = 20, height = 20, regardless of whether it is
> currently visible.  I mean, why not?  This isn't quantum physics!  We can
> know the state of our SVG without having to observe it!
>
> …
> 
> There may be technical reasons why this can't be done "right now" but the
> aim of this bug is to fix the issue, not sweep it under the carpet.  It
> makes absolutely no logical sense at all to claim that returning 'undefined'
> (or anything other than the correct dimensions) is either correct,
> appropriate or desirable.

I agree for your rectangle example or other shapes, but I think the underlying issue here really is text nodes.  Both font face and size determine the boundaries of the rendered node. Those properties can be inherited as CSS properties from a parent node when attached, and are therefore unknown in an unattached state. So the value for getBBox() on an unattached svg node is only known if it is not a text node and contains no child text nodes.

The browser could therefore check (at whatever performance cost) for child text nodes and return the "expected" value if none are found, but does anyone really want a method that will unpredictably return accurate values or an error (or zeros, or undefined…) depending on its child nodes?  That seems worse than what we have now.

In any case I think throwing an error is a mistake for executing a native method that takes no arguments and does not change state on a valid node. So I still think it is sensible and desirable to acknowledge that accurate values are unavailable and return `undefined`, without an error.
There are trivial cases for which this is easy to solve. If your application consists of such trivial cases it should be easy to figure out the bounding box yourself.

There are much more complicated cases e.g. paths with cubic splines, percentage units, skewed ellipses, text.

We're not going to provide a partial solution for trivial cases as that will simply confuse things. We are slowly converting the bounding box code to work without requiring rendering but this needs to be done for each element type. If and when that effort completes we could switch to a getBBox that works with non-rendered text. If you'd like to help provide C++ code to determine the bounding box of an ellipse subject to an arbitrary 2D transform for instance then this effort will go more quickly. Raise another bug if you wish to do that.

Changing the error value might break sites that currently work around it without providing any value so we won't be doing that. We need to return a rect object because that's how the API is defined so 'undefined' is out too and that would also break sites.
(In reply to Jonathan Gala from comment #38)
> Both font face and size determine the boundaries of the rendered
> node. Those properties can be inherited as CSS properties from a
> parent node when attached, and are therefore unknown in an unattached
> state.

This is the part I disagree with.

They can *change* when attached, as new rules may come into play, but they are not unknown.

If I load a .svg file, then all the information to render it is there.  If I create an SVG node from the contents of that file, then all the information to render it is still there.  Attaching it to the DOM *may* change the layout, but it doesn't mean it doesn't have a layout in its unattached state, otherwise .svg would be a useless graphics-interchange format.

Also, there are two separate issues here (possibly caused by the same underlying technical issue):
1) getBBox() doesn't work for non-attached nodes.
2) getBBox() doesn't work for attached but hidden nodes.

The latter is the much bigger issue in my view, and in this situation the rendering information is never 'unknown'.

The former, I concede, may have certain situations where it cannot return a sensible answer, but in that case I would expect it to be consistent with offsetWidth, which returns 0 if a text element is not in the DOM (not 'undefined' or an exception).

Perhaps another way of addressing this is to look at how other browsers handle it, as my understanding from comment 0 is that most of them do.

(In reply to Robert Longson from comment #39)
> We're not going to provide a partial solution for trivial cases as that will
> simply confuse things. We are slowly converting the bounding box code to
> work without requiring rendering but this needs to be done for each element
> type.

You're saying that there is no point in releasing fixes for certain node types until you've got it perfect for all node types.  That's a bit like saying you're not going to release any new HTML5 features until you've completed all of them, as it might 'confuse things'.  I don't buy that - I think we'll manage to cope with a less-breaky implementation in the absence of a completely non-breaky one.

> If you'd like to help provide C++ code to determine the bounding
> box of an ellipse subject to an arbitrary 2D transform for
> instance then this effort will go more quickly. Raise another
> bug if you wish to do that.

I'd love to, but unfortunately I'm not a C++ programmer.  Are you saying that Firefox currently can't handle this?  My understanding was that the issue is down to the values being calculated at point of render, and therefore unavailable when non-rendered.  I didn't realise that 2D transformations have not yet been implemented at all!

> If and when that effort completes we could switch to a getBBox that
> works with non-rendered text.

I understand that there might be a fair bit of work under-the-hood to enable Firefox to fix this, and I'm aware it may take some time to implement.  However, perfect is the enemy of good-enough, and I'm sure that if this bug could be replaced "getBBox() fails for rotated paths", instead of "getBBox() fails for everything" then that would be a good improvement!
Bugs are not for long discussions. Please take this to the svg newsgroup https://groups.google.com/forum/#!forum/mozilla.dev.tech.svg

To clear up one misconception. 2D transforms have been implemented, code to get the bounding box of such transformed shapes without asking the rendering engine to do it is in progress and that's what this bug would need.
Flags: webcompat?
Whiteboard: [webcompat]
Flags: webcompat?

Migrating Webcompat whiteboard priorities to project flags. See bug 1547409.

Webcompat Priority: --- → ?

See bug 1547409. Migrating whiteboard priority tags to program flags.

From a developer perspective, Firefox should just return zeroes like every other browser has for the past 9 years. and it also matches getBoundingClientRect().

document.createElementNS('http://www.w3.org/2000/svg', "rect").getBoundingClientRect();
// DOMRect { x: 0, y: 0, width: 0, height: 0, top: 0, right: 0, bottom: 0, left: 0 }
document.createElementNS('http://www.w3.org/2000/svg', "rect").getBBox();
// Every other browser: SVGRect {x: 0, y: 0, width: 0, height: 0}
// Firefox: [Exception... "Failure"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: debugger eval code :: <TOP_LEVEL> :: line 1"  data: no]

Cameron, could you take a look at this longstanding compat issue?

Flags: needinfo?(cam)
Webcompat Priority: ? → P3
Assignee: nobody → emilio
Status: NEW → ASSIGNED

Thanks for snatching this one Emilio.

Flags: needinfo?(cam)
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/de7d485e3a0f
Make .getBBox() on a non-rendered element not throw. r=heycam
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/23038 for changes under testing/web-platform/tests
Whiteboard: [webcompat] → [webcompat], [wptsync upstream]
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla77
Upstream PR merged by moz-wptsync-bot
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: