835048 - Figure out if SVGElement.ownerSVGElement should throw

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

The spec (<https://svgwg.org/svg2-draft/types.html#__svg__SVGElement__ownerSVGElement>) says:

ownerSVGElement (readonly SVGSVGElement)
    The nearest ancestor ‘svg’ element. Null if the given element is the outermost svg element.

Our implementation has:

1138   // If we didn't find anything and we're not the outermost SVG element,
1139   // we've got an invalid structure
1140   if (!ownerSVGElement && Tag() != nsGkAtoms::svg) {
1141     rv.Throw(NS_ERROR_FAILURE);
1142   }

which doesn't seem to make sense to me.

Comment 1

[Robert Longson [:longsonr]]

It throws if you write <g xmlns="xmlns="http://www.w3.org/2000/svg"><rect id="r"></g> which is not a valid svg file as the root element is not <svg>.

Since the specification doesn't say to throw and returning null is reasonable I think we should do that.

Comment 2

[Robert Longson [:longsonr]]

Attachment: patch

Comment 3

[Jonathan Watt [:jwatt]]

What do Chrome and IE do?

Comment 4

[Robert Longson [:longsonr]]

Attachment: testcase

Firefox throws and displays an error in the console
IE 9 prints null
Opera prints null

Dunno about Chrome.

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

null in Chrome too.

Comment 6

[Jonathan Watt [:jwatt]]

Comment on attachment 785522 [details] [diff] [review]
patch

Okay, sounds like we should definitely do this then. Thanks!

Comment 7

[Jonathan Watt [:jwatt]]

Can you change:

  return GetCtx();

to:

  return GetCtx(); // this may return nullptr

Comment 8

[Robert Longson [:longsonr]]

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

Comment 9

[Robert Longson [:longsonr]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bd5a82670cf9

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bd5a82670cf9