User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:55.0) Gecko/20100101 Firefox/55.0 Build ID: 20170502030211 Steps to reproduce: Use attachment or do the following: Use svg: <svg id="SvgjsSvg1002" width="1500" height="750" xmlns="http://www.w3.org/2000/svg" version="1.1" xmlns:xlink="http://www.w3.org/1999/xlink" viewBox="0 0 1500 750"> <defs id="SvgjsDefs1003"></defs> <symbol id="SvgjsSymbol1009"> <rect id="SvgjsRect1010" width="100" height="100" fill="#cccccc" x="10" y="10"></rect> </symbol> <use id="SvgjsUse1011" xlink:href="#SvgjsSymbol1009" x="100" y="100"></use> </svg> Execute in console: `document.getElementById('SvgjsUse1011').getBBox()` Actual results: It returns {x:10,y:10 ...} Expected results: It should return {x:110, y:110 ...} There was a similar bug un chrome which is fixed already: https://bugs.chromium.org/p/chromium/issues/detail?id=512081 There also is a nice list with test cases in the specs which also cover this usecase: https://svgwg.org/svg2-draft/single-page.html#coords-BoundingBoxes

This is a breaking change in SVG 2 from SVG 1.1. In the SVG 1.1 version of the specification what we return is correct.

My understanding of a bounding box is, that it fits the shape in current user space. If I get the bbox of a use element and add it as rectangle to the parent element it should fully enclose the use element because they are in the same user space. So even according to SVG 1.1 I think that the above should be returned. Any other interpretation kinda renders the bbox useless (correct me if iam wrong but I cant see a usecase for a use-bbox defined like this).

In SVG 1.1 the <use> x,y became a transform and bounding boxes are returned within the transformed co-ordinate space.

The patch fixes bounding boxes but breaks clipping, masking and filtering of use elements. https://treeherder.mozilla.org/#/jobs?repo=try&revision=e69fccf119e94f1a96e43a7cef5e10a09c56681c&selectedJob=111401090

Perhaps bug 537623 fixed the wrong extra translation?

relative bug on chomium: https://bugs.chromium.org/p/chromium/issues/detail?id=512081

Comment on attachment 8902510 [details] Bug 1361639 - Part 2. Add test cases of calling getBBox on use element in test_bbox.xhtml. https://reviewboard.mozilla.org/r/174088/#review179596 r+ assuming issues are fixed. ::: dom/svg/test/bbox-helper.svg:2 (Diff revision 2) > <?xml version="1.0"?> > -<svg xmlns="http://www.w3.org/2000/svg"> > +<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"> This change is not necessary (see below) ::: dom/svg/test/bbox-helper.svg:38 (Diff revision 2) > <circle cx="100" cy="100" r="5"/> > <g/> > <circle cx="100" cy="100" r="5"/> > <g/> > </g> > + <use x="100" y="100" id="use_v" xlink:href="#v"/> just use href rather than xlink:href

Comment on attachment 8902511 [details] Bug 1361639 - Part 3. Remove unused EffectOffsets::offsetToBoundingBoxInDevPx. https://reviewboard.mozilla.org/r/174090/#review179600

Comment on attachment 8902231 [details] Bug 1361639 - Part 1. Depend on input flag, return bbox of an <use> element in different coordinate system. https://reviewboard.mozilla.org/r/173752/#review179602 r+ assuming nits are fixed. ::: dom/svg/SVGTransformableElement.cpp:192 (Diff revision 5) > return nullptr; > } > > if (!NS_SVGNewGetBBoxEnabled()) { > - return NS_NewSVGRect(this, ToRect(nsSVGUtils::GetBBox(frame))); > + return NS_NewSVGRect(this, ToRect(nsSVGUtils::GetBBox(frame, > + nsSVGUtils::eBBoxIncludeFillGeometry | Don't think you need to (or should) pass nsSVGUtils::eBBoxIncludeFillGeometry here. ::: layout/svg/nsSVGUtils.h:397 (Diff revision 5) > eBBoxIncludeFillGeometry = 1 << 1, > eBBoxIncludeStroke = 1 << 2, > eBBoxIncludeStrokeGeometry = 1 << 3, > eBBoxIncludeMarkers = 1 << 4, > eBBoxIncludeClipped = 1 << 5, > + // This flag takes effect on if getBBox calls on outer-<svg>. This flag takes effect on outer-<svg> elements. (Although given the name of the flag I don't think this comment adds much so I'd be tempted not to make this change at all). ::: layout/svg/nsSVGUtils.h:408 (Diff revision 5) > eForGetClientRects = 1 << 7, > // If the given frame is an HTML element, only include the region of the > // given frame, instead of all continuations of it, while computing bbox if > // this flag is set. > eIncludeOnlyCurrentFrameForNonSVGElement = 1 << 8, > + // This flag takes effect only if getBBox calls on <use>. This flag is only has an effect when the target is a <use> element. ::: layout/svg/nsSVGUtils.h:411 (Diff revision 5) > // this flag is set. > eIncludeOnlyCurrentFrameForNonSVGElement = 1 << 8, > + // This flag takes effect only if getBBox calls on <use>. > + // getBBox returns the bounds of the elements children in user space if > + // this flag is set; Otherwise, getBBox returns the union bounds in > + // coordinate system formed by <use> element. the coordinate system formed by the <use> element.

Comment on attachment 8902231 [details] Bug 1361639 - Part 1. Depend on input flag, return bbox of an <use> element in different coordinate system. https://reviewboard.mozilla.org/r/173752/#review179602 > Don't think you need to (or should) pass nsSVGUtils::eBBoxIncludeFillGeometry here. nsSVGUtils::GetBBox calls and pass aFlags to SVGGeometryFrame::GetBBoxContribution. In SVGGeometryFrame::GetBBoxContribution, we do need nsSVGUtils::eBBoxIncludeFillGeometry flag to inlcude fill geometry[1]. http://searchfox.org/mozilla-central/source/layout/svg/SVGGeometryFrame.cpp#498

Pushed by cku@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/24bcdb71be27 Part 1. Depend on input flag, return bbox of an <use> element in different coordinate system. r=longsonr+218550 https://hg.mozilla.org/integration/autoland/rev/14ca951b7d37 Part 2. Add test cases of calling getBBox on use element in test_bbox.xhtml. r=longsonr+218550 https://hg.mozilla.org/integration/autoland/rev/f73863428c5c Part 3. Remove unused EffectOffsets::offsetToBoundingBoxInDevPx. r=longsonr+218550

https://hg.mozilla.org/mozilla-central/rev/24bcdb71be27 https://hg.mozilla.org/mozilla-central/rev/14ca951b7d37 https://hg.mozilla.org/mozilla-central/rev/f73863428c5c