getBoundingClientRect gives odd results for <svg>

RESOLVED FIXED in mozilla33

Status

()

Core
SVG
RESOLVED FIXED
8 years ago
a year ago

People

(Reporter: bz, Assigned: roc)

Tracking

Trunk
mozilla33
x86
Mac OS X
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 4 obsolete attachments)

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>....
Created attachment 414437 [details]
Testcase; should probably show 200
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>?
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.
Created attachment 414570 [details] [diff] [review]
patch for outer-<svg>
Attachment #414570 - Flags: review?(bzbarsky)
I think nsISVGChildFrame may be misnamed. I'll need to look at the code more closely at some point.
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?
Attachment #414570 - Flags: review?(bzbarsky) → review+
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.
Is there a workout that can be used for Firebug for current shipping versions of Fx?
The patch seems consistent with bug 530985 comment 1
Duplicate of this bug: 509603
Duplicate of this bug: 479058
bug 479058 also has interesting information on this topic
Duplicate of this bug: 786269
Duplicate of this bug: 874828
Created attachment 766427 [details] [diff] [review]
patch with tests

Also does inner svg elements.
Assignee: nobody → longsonr
Attachment #766427 - Flags: review?(roc)
Attachment #766427 - Flags: review?(roc) → review+
Flags: in-testsuite+
And backed out http://hg.mozilla.org/integration/mozilla-inbound/rev/37cc02d2b65f for breaking tests.
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.
Created attachment 781658 [details]
A simple 40px diameter circle SVG

Used to render the circle through an object tag.
Created attachment 781661 [details]
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 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.
Attachment #781661 - Attachment is obsolete: true
Created attachment 781663 [details]
Hopefully a test to show embedded object SVG with wrong dimensions

How bugzilla didn't detect HTML, I dunno.
Comment on attachment 781663 [details]
Hopefully a test to show embedded object SVG with wrong dimensions

Ugh. One more time is a charm.
Attachment #781663 - Attachment is obsolete: true
Created attachment 781664 [details]
Hopefully a test to show embedded object SVG with wrong dimensions
Attachment #781664 - Attachment mime type: application/octet-stream → text/html
Created attachment 8351566 [details]
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?
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?
And cc me on that bug, please?
Assignee: longsonr → nobody

Comment 30

4 years ago
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?
See comment 19. Some XUL test fails with the patch. I think the patch exposes an existing XUL/SVG sizing bug.
Comment hidden (spam)

Comment 33

4 years ago
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.
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.
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.
I'll unbitrot the patch.
Thanks, but I've already done that! I'm on this :-)
Assignee: nobody → roc
Yeah, Chrome returns 45 for that case, basically just using the CSS border-box. So I think we should do the same for nsSVGOuterSVGFrame.
OK, I'll leave you to it.
Created attachment 8451346 [details] [diff] [review]
530985
Attachment #414570 - Attachment is obsolete: true
Attachment #766427 - Attachment is obsolete: true
Attachment #8451346 - Flags: review?(jwatt)
Comment on attachment 8451346 [details] [diff] [review]
530985

thank you
Attachment #8451346 - Flags: review?(jwatt) → review+
I'm guessing we'll need a follow-up bug for outer-<svg>.
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.
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.
https://hg.mozilla.org/mozilla-central/rev/99e4b83dd83e
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Thanks Roc.
Blocks: 342513
(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.
Blocks: 1049050
Blocks: 1073909
Comment hidden (off-topic)
Comment hidden (obsolete)
You need to log in before you can comment on or make changes to this bug.