SVG layout changes should be flushed before DOM calls

RESOLVED FIXED

Status

()

Core
SVG
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Robert Longson, Assigned: Robert Longson)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

11 years ago
If the layout is updated and DOM functions are called the functions will return data appropriate to the old rather than the updated layout.
(Assignee)

Comment 1

11 years ago
Created attachment 223918 [details] [diff] [review]
flush layout changes before calling DOM functions

As discussed in mozilla.tech.dev.svg group

Bug .getBBox() and .getCharNumAtPosition() after modifiying text-anchor attribute
Assignee: general → longsonr
Status: NEW → ASSIGNED
Attachment #223918 - Flags: review?(tor)
> +    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.
(Assignee)

Comment 3

11 years ago
Created attachment 223933 [details] [diff] [review]
address bz's comments
Attachment #223918 - Attachment is obsolete: true
Attachment #223933 - Flags: review?(tor)
Attachment #223918 - Flags: review?(tor)
(Assignee)

Comment 4

11 years ago
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.

Updated

11 years ago
Attachment #223933 - Flags: review?(tor) → review+
(Assignee)

Updated

11 years ago
Attachment #223933 - Flags: superreview?(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.
Attachment #223933 - Flags: superreview?(bzbarsky) → superreview+
(Assignee)

Comment 6

11 years ago
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
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.