530985 - getBoundingClientRect gives odd results for <svg>

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Boris Zbarsky [:bzbarsky]]

nsSVGOuterSVGFrame QIs to nsISVGChildFrame (should it?).  As a result, nsSVGUtils::GetOuterSVGFrameAndCoveredRegion on it returns itself and puts the covered region into aRect.  This is what we use in BoxToBorderRect::AddBox, so we end up using the covered region as the getBoundingClientRect.

For outer <svg> that seems wrong to me.  Heck, it doesn't seem all that right even for inner <svg>....

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase; should probably show 200

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

It's a bug for outer <svg>, but not for inner <svg> according to the CSSOM Views spec (which I agree with).

http://www.w3.org/TR/cssom-view/#the-getclientrects-and-getboundingclient

What did you think should be returned for inner <svg>?

Comment 3

[Boris Zbarsky [:bzbarsky]]

I would have thought that returning the rect for the viewport it establishes might have made sense; I can live with the svg bounding box, though.

I still think it's really weird that outer SVG QIs to nsISVGChildFrame, too.

Comment 4

[Jonathan Watt [:jwatt]]

Attachment: patch for outer-<svg>

Comment 5

[Jonathan Watt [:jwatt]]

I think nsISVGChildFrame may be misnamed. I'll need to look at the code more closely at some point.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 414570 [details] [diff] [review]
patch for outer-<svg>

This seems ok, I guess, but is the nsISVGChildFrame behavior actually correct?  If not, can we fix that instead?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Point being, that if we change nsISVGChildFrame we should, imo, back out this fix.

What we should also do is land a test as part of this fix.

Comment 8

[Steve Roussey (:sroussey)]

Is there a workout that can be used for Firebug for current shipping versions of Fx?

Comment 9

[Robert Longson [:longsonr]]

The patch seems consistent with bug 530985 comment 1

Comment 12

[Robert Longson [:longsonr]]

bug 479058 also has interesting information on this topic

Comment 15

[Robert Longson [:longsonr]]

Attachment: patch with tests

Also does inner svg elements.

Comment 16

[Robert Longson [:longsonr]]

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

Comment 17

[Robert Longson [:longsonr]]

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

Comment 18

[Robert Longson [:longsonr]]

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/24de4eda70e2

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

And backed out http://hg.mozilla.org/integration/mozilla-inbound/rev/37cc02d2b65f for breaking tests.

Comment 20

[[e-mail]]

I see odd behavior in the currently nightly when the SVG is rendered via an object tag. Looking at the example https://bugzilla.mozilla.org/attachment.cgi?id=414437 that says it probably ought to be reported as 200 but reports 40. If that's the expected behavior, then when I convert to an embedded object tag loading the SVG, I now get 200 for the SVG as a whole and 40 for the circle. I'll try and attach the two files.

Comment 21

[[e-mail]]

Attachment: A simple 40px diameter circle SVG

Used to render the circle through an object tag.

Comment 22

[[e-mail]]

Attachment: Hopefully a test to show embedded object SVG with wrong dimensions

This should show the circle SVG. Note that the SVG bounds are now reported as 200 but when the same SVG is embedded, it's reported as 40.

Comment 23

[[e-mail]]

Comment on attachment 781661 [details]
Hopefully a test to show embedded object SVG with wrong dimensions

Dang, didn't come through at HTML. Let me try again.

Comment 24

[[e-mail]]

Attachment: Hopefully a test to show embedded object SVG with wrong dimensions

How bugzilla didn't detect HTML, I dunno.

Comment 25

[[e-mail]]

Comment on attachment 781663 [details]
Hopefully a test to show embedded object SVG with wrong dimensions

Ugh. One more time is a charm.

Comment 26

[[e-mail]]

Attachment: Hopefully a test to show embedded object SVG with wrong dimensions

Comment 27

[Luís Miguel [:quicksaver] (not reading bugmail)]

Attachment: getClientRect test on an SVG

STR:

1) Open the "getClientRect test on an SVG" attachment on a tab (not on bugzilla's viewer, actually open the svg frame in its own tab)
2) With your mouse, select the "Findbar" text at the top of the svg
3) Without removing the text selection, open the browser console
4) Type in the console:
> gFindBar.browser.finder._getSelectionController(gFindBar.browser.contentWindow).getSelection(gFindBar.nsISelectionController.SELECTION_NORMAL).getRangeAt(0).getBoundingClientRect()

The console will report negative values for the top and bottom values. Is this because of the same bug as on this thread?

Comment 28

[Boris Zbarsky [:bzbarsky]]

gFindBar doesn't exist in nightlies, but if I do content.getSelection(1).getRangeAt(0).getBoundingClientRect() I also get negative top and bottom values.  

That's somewhat likely to be a different bug, though.  Could you please file it?

Comment 29

[Boris Zbarsky [:bzbarsky]]

And cc me on that bug, please?

Comment 30

[David Vest]

This bug limits automatic testing of inline SVGs as in http://w3c-test.org/html/rendering/replaced-elements/svg-inline-sizing/svg-inline.html in Firefox. With this bug patched, FF passes all 648 tests.

Anything I can do to help it along? Not sure what the back out was about. Flaky/invalid test or a flaw in the patch?

Comment 31

[Robert Longson [:longsonr]]

See comment 19. Some XUL test fails with the patch. I think the patch exposes an existing XUL/SVG sizing bug.

Comment 32

[[e-mail]]

So the "plan" is to ignore the root of the problem (the stupid XUL bug) even though the patch clearly fixes a boat load of SVG issues? Good to see Mozilla pushing the leading edge on this stuff :-( Back to Chrome!

Comment 33

[Gavin Kistner]

Gleah. Over four years since this was filed. Here's another test showing the grossness:
http://jsfiddle.net/dL5pZ/3/

And here's my Stack Overflow question trying to find any workaround to determine how large an SVG element is on the page.
http://stackoverflow.com/questions/23684821/calculate-size-of-svg-element-in-html-page

Even the developer tools show the incorrect size for the `<svg>` element.

Comment 34

[Robert O'Callahan (:roc) (email my personal email if necessary)]

These aren't XUL test failures. They're just in test_offsets.xul, which tests lots of things including HTML and SVG. And the test failures are simple: that file contains
<svg:svg id="svgbase" width="45" height="20" xmlns:svg="http://www.w3.org/2000/svg">
  <svg:rect id="svgrect" x="3" y="5" width="45" height="20" fill="red"/>
</svg:svg>
and expects the bounding rect to be the border-box of "svgbase", which is 45x20. But with the new patch its bounding rect is the SVG bbox, which is 48x25. So the new results are as expected.

Comment 35

[Robert O'Callahan (:roc) (email my personal email if necessary)]

One way to fix these failures would be to say that for the outermost <svg> element, getClientRects etc should return the border-box like other elements. The spec is unclear on this point. I need to check what other browsers do.

Comment 36

[Robert Longson [:longsonr]]

I'll unbitrot the patch.

Comment 37

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Thanks, but I've already done that! I'm on this :-)

Comment 38

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yeah, Chrome returns 45 for that case, basically just using the CSS border-box. So I think we should do the same for nsSVGOuterSVGFrame.

Comment 39

[Robert Longson [:longsonr]]

OK, I'll leave you to it.

Comment 40

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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

Comment 41

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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

Comment 42

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: 530985

Comment 43

[Jonathan Watt [:jwatt]]

Comment on attachment 8451346 [details] [diff] [review]
530985

thank you

Comment 44

[Jonathan Watt [:jwatt]]

I'm guessing we'll need a follow-up bug for outer-<svg>.

Comment 45

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/99e4b83dd83e

(In reply to Jonathan Watt [:jwatt] from comment #44)
> I'm guessing we'll need a follow-up bug for outer-<svg>.

I don't think so. This patch makes outer <svg> be treated as if it's a normal CSS-layout element, which is what we want I think.

Comment 46

[Jonathan Watt [:jwatt]]

Ah, great. We should add a few outer-<svg> tests similar to the inner-<svg> tests in the patch. r=me if you want to go ahead and do that, or else I'll knock something up tomorrow.

Comment 47

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/99e4b83dd83e

Comment 48

[Robert Longson [:longsonr]]

Thanks Roc.

Comment 49

[Jonathan Watt [:jwatt]]

(In reply to Jonathan Watt [:jwatt] from comment #46)
> Ah, great. We should add a few outer-<svg> tests similar to the inner-<svg>
> tests in the patch. r=me if you want to go ahead and do that, or else I'll
> knock something up tomorrow.

Done in bug 1049050.

Comment 50

[Kitana]

Hi!
node.clientWidth and node.clientHeight return 0
Are this case relate to bug?

fiddle:
https://jsfiddle.net/Kitana/0br8mLs6/4/
(reproduce in Firefox 48)

Though boundingClientRect returned right values.

Comment 51

[Robert Longson [:longsonr]]

(In reply to Kitana from comment #50)
> Hi!
> node.clientWidth and node.clientHeight return 0
> Are this case relate to bug?
> 

That's bug 874811 and is unrelated to this bug