Make svg use new attribute code

RESOLVED FIXED

Status

()

RESOLVED FIXED
15 years ago
14 years ago

People

(Reporter: sicking, Assigned: sicking)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

The new attributecode is now ready enough to convert SVG to start using it.

The following should be done:
* Make nsSVGElement::GetAttributes just forward to nsGenericElement.
  This means that svg won't have to bother with creating attributenodes or
  implement an nsIDOMNamedNodeMap for attributes at all.
* Store all nsISVGValues in mAttrsAndChildren (the member already exists but is
  only used for children)
* Remove all code in nsSVGAttribute and nsSVGAttributes that implements 
  nsIDOMAttr resp. nsIDOMNamedNodeMap

Once this is done there is probably little reason for the
nsSVGAttribute/nsSVGAttributes classes to still exist. The remaining code and
members in nsSVGAttributes could probably be moved to nsSVGElement, and
nsSVGAttribute will probably more or less just disappear.
Created attachment 143332 [details] [diff] [review]
Patch to fix

The patch does the following:

* Store attributes in mAttrsAndChildren.
* Only attributes that are also available throuh dom-properties (such as .r)
are
  stored as nsISVGValues. All other attributes are just stored as strings.
* Use the code from nsGenericElement for a bunch of attribute-code, including:
    .attributes
    attribute-nodes
    HasAttr/GetAttr/UnsetAttr/GetAttrNameAt/GetAttrCount
* Send off nsIDocumentObserver and mutation-events when attributes are modified

  through dom-properties. This way a stylerule like
  circle[r="150"] { fill: red }
  will work when someone does myCircle.r.baseVal.value=150
* Kill nsISVGAttribute.h/nsSVGAttribute.h/nsSVGAttribute.cpp
* Remove code in element dtors that unregistered itself as observer of
  dom-properties. AFAICT in the old code the elements didn't actually observe
  the dom-properties. In the new code add the element as observer in
  nsSVGElement, so i'd like to remove it as observer there too.
Assignee: alex → bugmail
Status: NEW → ASSIGNED
Comment on attachment 143332 [details] [diff] [review]
Patch to fix

jst: could you r/sr the nsAttrValue.cpp|h part of this patch?
Attachment #143332 - Flags: superreview?(jst)
Comment on attachment 143332 [details] [diff] [review]
Patch to fix

sr=jst for the nsAttrValue part.
Attachment #143332 - Flags: superreview?(jst) → superreview+

Comment 4

15 years ago
Comment on attachment 143332 [details] [diff] [review]
Patch to fix

great stuff. r=afri
Attachment #143332 - Flags: review?(alex) → review+
checked in. Thanks for the quick reviews
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.