339807 - SVG layout changes should be flushed before DOM calls

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Robert Longson [:longsonr]]

If the layout is updated and DOM functions are called the functions will return data appropriate to the old rather than the updated layout.

Comment 1

[Robert Longson [:longsonr]]

Attachment: flush layout changes before calling DOM functions

As discussed in mozilla.tech.dev.svg group

Bug .getBBox() and .getCharNumAtPosition() after modifiying text-anchor attribute

Comment 2

[Boris Zbarsky [:bzbarsky]]

> +    NS_ERROR("no document");

Er... NO. Do NOT do that.  Having no current doc is a perfectly reasonable state of things for a DOM node.  Having a DOM API called on such node is also perfectly reasonable.  Asserting when this happens is NOT acceptable.  Furthermore, does the SVG spec really not define a better error code than NS_ERROR_FAILURE for this case?

> +    NS_ERROR("no presshell");

Again, this is NOT something you can assert.

I see that nsSVGPathElement, nsSVGTextElement, nsSVGTextPathElement, and nsSVGTSpanElement already have such bogus asserts.  Please remove them.

Comment 3

[Robert Longson [:longsonr]]

Attachment: address bz's comments

Comment 4

[Robert Longson [:longsonr]]

As far as I can tell the SVG spec does not define any failure codes except invalid argument values for it's own functions. Things like what happens if the element is display:none and you call a DOM function on it are not mentioned.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 223933 [details] [diff] [review]
address bz's comments

sr=bzbarsky, though this looks like it should perhaps be a GetPresShell method on some common superclass instead of copy/pasting the same code a bunch of times.

Comment 6

[Robert Longson [:longsonr]]

Checked in.

Checking in nsSVGGraphicElement.cpp;
/cvsroot/mozilla/content/svg/content/src/nsSVGGraphicElement.cpp,v  <--  nsSVGGraphicElement.cpp
new revision: 1.32; previous revision: 1.31
done
Checking in nsSVGPathElement.cpp;
/cvsroot/mozilla/content/svg/content/src/nsSVGPathElement.cpp,v  <--  nsSVGPathElement.cpp
new revision: 1.18; previous revision: 1.17
done
Checking in nsSVGSVGElement.cpp;
/cvsroot/mozilla/content/svg/content/src/nsSVGSVGElement.cpp,v  <--  nsSVGSVGElement.cpp
new revision: 1.69; previous revision: 1.68
done
Checking in nsSVGTSpanElement.cpp;
/cvsroot/mozilla/content/svg/content/src/nsSVGTSpanElement.cpp,v  <--  nsSVGTSpanElement.cpp
new revision: 1.22; previous revision: 1.21
done
Checking in nsSVGTextElement.cpp;
/cvsroot/mozilla/content/svg/content/src/nsSVGTextElement.cpp,v  <--  nsSVGTextElement.cpp
new revision: 1.25; previous revision: 1.24
done
Checking in nsSVGTextPathElement.cpp;
/cvsroot/mozilla/content/svg/content/src/nsSVGTextPathElement.cpp,v  <--  nsSVGTextPathElement.cpp
new revision: 1.6; previous revision: 1.5
done