Tracking bug for SVG on the 1.8 branch

RESOLVED FIXED

Status

()

Core
SVG
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: mkaply, Assigned: mkaply)

Tracking

({fixed1.8.1})

1.8 Branch
x86
All
fixed1.8.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(18 attachments, 1 obsolete attachment)

862 bytes, patch
Darin Fisher
: approval1.8.1+
Details | Diff | Splinter Review
3.28 KB, patch
Darin Fisher
: approval1.8.1+
Details | Diff | Splinter Review
1.19 KB, patch
Darin Fisher
: approval1.8.1+
Details | Diff | Splinter Review
3.53 KB, patch
Darin Fisher
: approval1.8.1+
Details | Diff | Splinter Review
1.10 KB, patch
Darin Fisher
: approval1.8.1+
Details | Diff | Splinter Review
1.45 KB, patch
Darin Fisher
: approval1.8.1+
Details | Diff | Splinter Review
1.58 KB, patch
Darin Fisher
: approval1.8.1+
Details | Diff | Splinter Review
8.83 KB, patch
Darin Fisher
: approval1.8.1+
Details | Diff | Splinter Review
4.58 KB, patch
Darin Fisher
: approval1.8.1+
Details | Diff | Splinter Review
9.34 KB, patch
Darin Fisher
: approval1.8.1+
Details | Diff | Splinter Review
1.65 KB, patch
Darin Fisher
: approval1.8.1+
Details | Diff | Splinter Review
6.39 KB, patch
Darin Fisher
: approval1.8.1+
Details | Diff | Splinter Review
2.71 KB, patch
Darin Fisher
: approval1.8.1+
Details | Diff | Splinter Review
3.58 KB, patch
Darin Fisher
: approval1.8.1+
Details | Diff | Splinter Review
9.17 KB, patch
Darin Fisher
: approval1.8.1+
Details | Diff | Splinter Review
1.10 KB, patch
Darin Fisher
: approval1.8.1+
Details | Diff | Splinter Review
14.39 KB, patch
Darin Fisher
: approval1.8.1+
Details | Diff | Splinter Review
1.92 KB, patch
tor
: review+
Details | Diff | Splinter Review
(Assignee)

Description

12 years ago
Rather than mess with the existing bugs, I'm going to attach patches here and seek approval here.
(Assignee)

Comment 1

12 years ago
Created attachment 223293 [details] [diff] [review]
310996 - We shouldn't assert in nsSVGEnum::SetValueString (checked in)
Assignee: general → mozilla
Status: NEW → ASSIGNED
Attachment #223293 - Flags: approval-branch-1.8.1?
(Assignee)

Comment 2

12 years ago
Created attachment 223294 [details] [diff] [review]
311031 - implement SVGTSpanElement.getComputedTextLength

DOM compliance
Attachment #223294 - Flags: approval-branch-1.8.1?
(Assignee)

Comment 3

12 years ago
Created attachment 223296 [details] [diff] [review]
311063 - Bogus assertions, and tspan attributes x/y/dx/dy aren't live (checked in)

DOM compliance issue, low risk.
Attachment #223296 - Flags: approval-branch-1.8.1+
(Assignee)

Comment 4

12 years ago
Created attachment 223297 [details] [diff] [review]
312269 - marker leaves poopies along the 4 cardinal directions (on my machine anyway) (checked in)

Low risk - visual bug
Attachment #223297 - Flags: approval-branch-1.8.1?
(Assignee)

Comment 5

12 years ago
Created attachment 223298 [details] [diff] [review]
312376 - Repair textPath from bug 311063 (checked in)

Fallout from above checkin for 311063
Attachment #223298 - Flags: approval-branch-1.8.1?
(Assignee)

Updated

12 years ago
Attachment #223296 - Flags: approval-branch-1.8.1+ → approval-branch-1.8.1?
(Assignee)

Comment 6

12 years ago
Created attachment 223299 [details] [diff] [review]
315861 - SVG Mouse events inhibited by clippath in proximity (checked in)

Low risk bug fix
Attachment #223299 - Flags: approval-branch-1.8.1?
(Assignee)

Comment 7

12 years ago
Created attachment 223300 [details] [diff] [review]
317709 - onclick does not work with textPath SVG elements (checked in)

Low risk fix
Attachment #223300 - Flags: approval-branch-1.8.1?
(Assignee)

Comment 8

12 years ago
Created attachment 223301 [details] [diff] [review]
318379 - Firefox 1.5 should not crash when viewing an SVG file, or a page embedding that file (checked in)

Crash fix
Attachment #223301 - Flags: approval-branch-1.8.1?
(Assignee)

Comment 9

12 years ago
Created attachment 223302 [details] [diff] [review]
320623 - overflow="visible" doesn't work on <marker> (checked in)

Correctness fix
Attachment #223302 - Flags: approval-branch-1.8.1?
(Assignee)

Comment 10

12 years ago
Created attachment 223303 [details] [diff] [review]
322922 - We have interface map entries for nsSupportsWeakReference! (checked in)

This was just bad code
Attachment #223303 - Flags: approval-branch-1.8.1?
(Assignee)

Comment 11

12 years ago
Created attachment 223306 [details] [diff] [review]
317027 - Classinfo is incorrect for some SVG nodes (checked in)

Correctness
Attachment #223306 - Flags: approval-branch-1.8.1?
(Assignee)

Comment 12

12 years ago
Created attachment 223307 [details] [diff] [review]
323589 - Trying to access .style of <svg:marker> throws "Component does not have requested interface" (NS_NOINTERFACE) (checked in)

Correctness fix
Attachment #223307 - Flags: approval-branch-1.8.1?
(Assignee)

Comment 13

12 years ago
Created attachment 223308 [details] [diff] [review]
323669 - Percent attribute values in userSpace gradients don't work (checked in)

bug
Attachment #223308 - Flags: approval-branch-1.8.1?
(Assignee)

Comment 14

12 years ago
Created attachment 223309 [details] [diff] [review]
323704 - Circular reference in 'clipPath' crashes browser instantly (checked in)

Recursive loop crash
Attachment #223309 - Flags: approval-branch-1.8.1?
(Assignee)

Comment 15

12 years ago
Created attachment 223310 [details] [diff] [review]
323753 - initialise meetOrSlice variable properly (checked in)

wrong variable initialized/set
Attachment #223310 - Flags: approval-branch-1.8.1?
(Assignee)

Comment 16

12 years ago
Created attachment 223311 [details] [diff] [review]
327437 - createSVGAngle isn't implemented (checked in)

correctness
Attachment #223311 - Flags: approval-branch-1.8.1?
(Assignee)

Comment 17

12 years ago
Created attachment 223314 [details] [diff] [review]
328137 - "ASSERTION: not a container" making one <svg:stop> a child of another (checked in)

fix assertion
Attachment #223314 - Flags: approval-branch-1.8.1?
(Assignee)

Comment 18

12 years ago
Created attachment 223323 [details] [diff] [review]
325728 - changes to markerWidth/markerHeight and orient not handled properly (checked in)

correctness.
Attachment #223323 - Flags: approval-branch-1.8.1?

Comment 19

12 years ago
How about bug 318597.
IMHO, It may help svg widget.
(Assignee)

Comment 20

12 years ago
(In reply to comment #19)
> How about bug 318597.
> IMHO, It may help svg widget.
> 

That bug is on my list. Because it is a large patch, I'm waiting until some other stuff gets in. Here are the other bugs we are considering:

bug 310436
bug 314627
bug 318597
bug 329758
bug 336451
bug 318597 has a problem c.f. bug 339039

bug 314627 is a bit of a bodge. Most of its code was rewritten in bug 318597. It was one of my earliest efforts. It makes nsTSpanElement an nsGraphicElement which it really shouldn't be. bug 318597 reverses this.
(Assignee)

Comment 22

12 years ago
(In reply to comment #21)
> bug 318597 has a problem c.f. bug 339039
> 
> bug 314627 is a bit of a bodge. Most of its code was rewritten in bug 318597.
> It was one of my earliest efforts. It makes nsTSpanElement an nsGraphicElement
> which it really shouldn't be. bug 318597 reverses this.
> 

Obviously, since you are the creator of these patches, I would defer to you for the right thing to do.

What do you think we should do with these three bugs on the 1.8 branch?
I am working on patch for bug 339039. That should be ready for review Tue/Wed next week.

I would recommend either taking all of  bug 314627, bug 318597 and bug 339039 or none. This depends on the timescales as bug 339039 would require some baking time.
(Assignee)

Comment 24

12 years ago
Shaver - this is the list of SVG things for 1.8
(In reply to comment #22)
> (In reply to comment #21)
> > bug 318597 has a problem c.f. bug 339039
> > 
> > bug 314627 is a bit of a bodge. Most of its code was rewritten in bug 318597.
> > It was one of my earliest efforts. It makes nsTSpanElement an nsGraphicElement
> > which it really shouldn't be. bug 318597 reverses this.
> > 
> 
> Obviously, since you are the creator of these patches, I would defer to you for
> the right thing to do.
> 
> What do you think we should do with these three bugs on the 1.8 branch?
> 

I have now checked in bug 339039 which fixes issues with bug 318597.

Your mileage may vary but the trunk has had a number of SVG infrastructure changes and integrating bugs 318597 and bug 339039 into a branch will not be straightforward as they are both large and therefore undoubtedly depend on various other trunk fixes. I have tried to update the dependencies of these bugs with the things I know about but that's bound to be incomplete.

If I were in your position, I'd probably reluctantly omit bug 314627, bug 318597 and bug 339039 for the above reason. I remember Tor's list of patches he integrated for textPath on the branch and that only just missed the cut for FF 1.5.

Having said that, I'd certainly like to see this functionality appear before FF 3 so if you have the time and inclination I'll try to help if I can.


Updated

12 years ago
Attachment #223293 - Flags: approval-branch-1.8.1? → approval1.8.1?

Updated

12 years ago
Attachment #223294 - Flags: approval-branch-1.8.1? → approval1.8.1?

Updated

12 years ago
Attachment #223296 - Flags: approval-branch-1.8.1? → approval1.8.1?

Updated

12 years ago
Attachment #223297 - Flags: approval-branch-1.8.1? → approval1.8.1?

Updated

12 years ago
Attachment #223298 - Flags: approval-branch-1.8.1? → approval1.8.1?

Updated

12 years ago
Attachment #223299 - Flags: approval-branch-1.8.1? → approval1.8.1?

Updated

12 years ago
Attachment #223300 - Flags: approval-branch-1.8.1? → approval1.8.1?

Updated

12 years ago
Attachment #223301 - Flags: approval-branch-1.8.1? → approval1.8.1?

Updated

12 years ago
Attachment #223302 - Flags: approval-branch-1.8.1? → approval1.8.1?

Updated

12 years ago
Attachment #223303 - Flags: approval-branch-1.8.1? → approval1.8.1?

Updated

12 years ago
Attachment #223306 - Flags: approval-branch-1.8.1? → approval1.8.1?

Updated

12 years ago
Attachment #223307 - Flags: approval-branch-1.8.1? → approval1.8.1?

Updated

12 years ago
Attachment #223308 - Flags: approval-branch-1.8.1? → approval1.8.1?

Updated

12 years ago
Attachment #223309 - Flags: approval-branch-1.8.1? → approval1.8.1?

Updated

12 years ago
Attachment #223310 - Flags: approval-branch-1.8.1? → approval1.8.1?

Updated

12 years ago
Attachment #223311 - Flags: approval-branch-1.8.1? → approval1.8.1?

Updated

12 years ago
Attachment #223314 - Flags: approval-branch-1.8.1? → approval1.8.1?

Updated

12 years ago
Attachment #223323 - Flags: approval-branch-1.8.1? → approval1.8.1?

Updated

12 years ago
Attachment #223293 - Flags: approval1.8.1? → approval1.8.1+
Comment on attachment 223294 [details] [diff] [review]
311031 - implement SVGTSpanElement.getComputedTextLength

This code was changed significantly in bug 318597.  Is this just a dummy implementation or is it really something close enough to what we have on the trunk that it's ok that authors depend on the difference?
(Assignee)

Comment 28

12 years ago
clickable view

bug 310996 - We shouldn't assert in nsSVGEnum::SetValueString
bug 311031 - implement SVGTSpanElement.getComputedTextLength
bug 311063 - Bogus assertions, and tspan attributes x/y/dx/dy aren't live
bug 312269 - marker leaves poopies along the 4 cardinal directions (on my machine anyway)
bug 312376 - Repair textPath from bug 311063
bug 315861 - SVG Mouse events inhibited by clippath in proximity
bug 317709 - onclick does not work with textPath SVG elements
bug 318379 - Firefox 1.5 should not crash when viewing an SVG file, or a page embedding that file
bug 320623 - overflow="visible" doesn't work on <marker>
bug 322922 - We have interface map entries for nsSupportsWeakReference!
bug 317027 - Classinfo is incorrect for some SVG nodes
bug 323589 - Trying to access .style of <svg:marker> throws 'Component does not have requested interface' (NS_NOINTERFACE)
bug 323669 - Percent attribute values in userSpace gradients don't work
bug 323704 - Circular reference in 'clipPath' crashes browser instantly
bug 323753 - initialise meetOrSlice variable properly
bug 327437 - createSVGAngle isn't implemented
bug 328137 - "ASSERTION: not a container" making one <svg:stop> a child of another<
bug 325728 - changes to markerWidth/markerHeight and orient not handled properly

Comment 29

12 years ago
Comment on attachment 223297 [details] [diff] [review]
312269 - marker leaves poopies along the 4 cardinal directions (on my machine anyway) (checked in)

a=darin on behalf of drivers (please land on the MOZILLA_1_8_BRANCH and add the fixed1.8.1 keyword to the bug)
Attachment #223297 - Flags: approval1.8.1? → approval1.8.1+

Comment 30

12 years ago
Comment on attachment 223299 [details] [diff] [review]
315861 - SVG Mouse events inhibited by clippath in proximity (checked in)

a=darin on behalf of drivers (please land on the MOZILLA_1_8_BRANCH and add the fixed1.8.1 keyword to the bug)
Attachment #223299 - Flags: approval1.8.1? → approval1.8.1+

Comment 31

12 years ago
Comment on attachment 223300 [details] [diff] [review]
317709 - onclick does not work with textPath SVG elements (checked in)

a=darin on behalf of drivers (please land on the MOZILLA_1_8_BRANCH and add the fixed1.8.1 keyword to the bug)
Attachment #223300 - Flags: approval1.8.1? → approval1.8.1+

Comment 32

12 years ago
Comment on attachment 223301 [details] [diff] [review]
318379 - Firefox 1.5 should not crash when viewing an SVG file, or a page embedding that file (checked in)

a=darin on behalf of drivers (please land on the MOZILLA_1_8_BRANCH and add the fixed1.8.1 keyword to the bug)
Attachment #223301 - Flags: approval1.8.1? → approval1.8.1+

Comment 33

12 years ago
Comment on attachment 223302 [details] [diff] [review]
320623 - overflow="visible" doesn't work on <marker> (checked in)

a=darin on behalf of drivers (please land on the MOZILLA_1_8_BRANCH and add the fixed1.8.1 keyword to the bug)
Attachment #223302 - Flags: approval1.8.1? → approval1.8.1+

Comment 34

12 years ago
Comment on attachment 223303 [details] [diff] [review]
322922 - We have interface map entries for nsSupportsWeakReference! (checked in)

a=darin on behalf of drivers (please land on the MOZILLA_1_8_BRANCH and add the fixed1.8.1 keyword to the bug)
Attachment #223303 - Flags: approval1.8.1? → approval1.8.1+

Comment 35

12 years ago
Comment on attachment 223306 [details] [diff] [review]
317027 - Classinfo is incorrect for some SVG nodes (checked in)

a=darin on behalf of drivers (please land on the MOZILLA_1_8_BRANCH and add the fixed1.8.1 keyword to the bug)
Attachment #223306 - Flags: approval1.8.1? → approval1.8.1+

Comment 36

12 years ago
Comment on attachment 223307 [details] [diff] [review]
323589 - Trying to access .style of <svg:marker> throws "Component does not have requested interface" (NS_NOINTERFACE) (checked in)

a=darin on behalf of drivers (please land on the MOZILLA_1_8_BRANCH and add the fixed1.8.1 keyword to the bug)
Attachment #223307 - Flags: approval1.8.1? → approval1.8.1+

Comment 37

12 years ago
Comment on attachment 223308 [details] [diff] [review]
323669 - Percent attribute values in userSpace gradients don't work (checked in)

a=darin on behalf of drivers (please land on the MOZILLA_1_8_BRANCH and add the fixed1.8.1 keyword to the bug)
Attachment #223308 - Flags: approval1.8.1? → approval1.8.1+

Comment 38

12 years ago
Comment on attachment 223309 [details] [diff] [review]
323704 - Circular reference in 'clipPath' crashes browser instantly (checked in)

a=darin on behalf of drivers (please land on the MOZILLA_1_8_BRANCH and add the fixed1.8.1 keyword to the bug)
Attachment #223309 - Flags: approval1.8.1? → approval1.8.1+

Comment 39

12 years ago
Comment on attachment 223310 [details] [diff] [review]
323753 - initialise meetOrSlice variable properly (checked in)

a=darin on behalf of drivers (please land on the MOZILLA_1_8_BRANCH and add the fixed1.8.1 keyword to the bug)
Attachment #223310 - Flags: approval1.8.1? → approval1.8.1+

Comment 40

12 years ago
Comment on attachment 223311 [details] [diff] [review]
327437 - createSVGAngle isn't implemented (checked in)

a=darin on behalf of drivers (please land on the MOZILLA_1_8_BRANCH and add the fixed1.8.1 keyword to the bug)
Attachment #223311 - Flags: approval1.8.1? → approval1.8.1+

Comment 41

12 years ago
Comment on attachment 223314 [details] [diff] [review]
328137 - "ASSERTION: not a container" making one <svg:stop> a child of another (checked in)

a=darin on behalf of drivers (please land on the MOZILLA_1_8_BRANCH and add the fixed1.8.1 keyword to the bug)
Attachment #223314 - Flags: approval1.8.1? → approval1.8.1+

Comment 42

12 years ago
Comment on attachment 223323 [details] [diff] [review]
325728 - changes to markerWidth/markerHeight and orient not handled properly (checked in)

a=darin on behalf of drivers (please land on the MOZILLA_1_8_BRANCH and add the fixed1.8.1 keyword to the bug)
Attachment #223323 - Flags: approval1.8.1? → approval1.8.1+

Comment 43

12 years ago
Comment on attachment 223296 [details] [diff] [review]
311063 - Bogus assertions, and tspan attributes x/y/dx/dy aren't live (checked in)

a=darin on behalf of drivers (please land on the MOZILLA_1_8_BRANCH and add the fixed1.8.1 keyword to the bug)
Attachment #223296 - Flags: approval1.8.1? → approval1.8.1+

Comment 44

12 years ago
Comment on attachment 223298 [details] [diff] [review]
312376 - Repair textPath from bug 311063 (checked in)

a=darin on behalf of drivers (please land on the MOZILLA_1_8_BRANCH and add the fixed1.8.1 keyword to the bug)
Attachment #223298 - Flags: approval1.8.1? → approval1.8.1+
(In reply to comment #27)
> (From update of attachment 223294 [details] [diff] [review] [edit])
> This code was changed significantly in bug 318597.  Is this just a dummy
> implementation or is it really something close enough to what we have on the
> trunk that it's ok that authors depend on the difference?
> 

It's a real implementation but it has a side effect that I eventually realised was undesirable, it changed tspan elements from being inherited from nsSVGStyleableElement to nsSVGGraphicalElement in order to use a method in that class. Unfortunately nsSVGGraphicalElements should be transformable and tspans are not. This was reverted as part of bug 318597.

As far as the different implementation goes, the trunk implementation should work with non rtl text while the branch implementation does not. This does not seem to have been a big problem as that's how text elements work on the branch currently.


(In reply to comment #45)
> It's a real implementation but it has a side effect that I eventually realised
> was undesirable,

Is it undesirable in the sense that it causes problems (including problems handling unexpected things, which could be potential security problems), or just undesirable from a code maintainance perspective?
(Assignee)

Updated

12 years ago
Attachment #223293 - Attachment description: 310996 - We shouldn't assert in nsSVGEnum::SetValueString → 310996 - We shouldn't assert in nsSVGEnum::SetValueString (checked in)
(Assignee)

Updated

12 years ago
Attachment #223296 - Attachment description: 311063 - Bogus assertions, and tspan attributes x/y/dx/dy aren't live → 311063 - Bogus assertions, and tspan attributes x/y/dx/dy aren't live (checked in)
(Assignee)

Updated

12 years ago
Attachment #223297 - Attachment description: 312269 - marker leaves poopies along the 4 cardinal directions (on my machine anyway) → 312269 - marker leaves poopies along the 4 cardinal directions (on my machine anyway) (checked in)
(Assignee)

Updated

12 years ago
Attachment #223298 - Attachment description: 312376 - Repair textPath from bug 311063 → 312376 - Repair textPath from bug 311063 (checked in)
(Assignee)

Updated

12 years ago
Attachment #223299 - Attachment description: 315861 - SVG Mouse events inhibited by clippath in proximity → 315861 - SVG Mouse events inhibited by clippath in proximity (checked in)
(Assignee)

Updated

12 years ago
Attachment #223300 - Attachment description: 317709 - onclick does not work with textPath SVG elements → 317709 - onclick does not work with textPath SVG elements (checked in)
(Assignee)

Updated

12 years ago
Attachment #223301 - Attachment description: 318379 - Firefox 1.5 should not crash when viewing an SVG file, or a page embedding that file → 318379 - Firefox 1.5 should not crash when viewing an SVG file, or a page embedding that file (checked in)
(Assignee)

Updated

12 years ago
Attachment #223303 - Attachment description: 322922 - We have interface map entries for nsSupportsWeakReference! → 322922 - We have interface map entries for nsSupportsWeakReference! (checked in)
(Assignee)

Updated

12 years ago
Attachment #223306 - Attachment description: 317027 - Classinfo is incorrect for some SVG nodes → 317027 - Classinfo is incorrect for some SVG nodes (checked in)
(Assignee)

Updated

12 years ago
Attachment #223302 - Attachment description: 320623 - overflow="visible" doesn't work on <marker> → 320623 - overflow="visible" doesn't work on <marker> (checked in)
(Assignee)

Updated

12 years ago
Attachment #223307 - Attachment description: 323589 - Trying to access .style of &lt;svg:marker&gt; throws &quot;Component does not have requested interface&quot; (NS_NOINTERFACE) → 323589 - Trying to access .style of <svg:marker> throws "Component does not have requested interface" (NS_NOINTERFACE) (checked in)
(Assignee)

Updated

12 years ago
Attachment #223309 - Attachment description: 323704 - Circular reference in 'clipPath' crashes browser instantly → 323704 - Circular reference in 'clipPath' crashes browser instantly (checked in)
(Assignee)

Updated

12 years ago
Attachment #223310 - Attachment description: 323753 - initialise meetOrSlice variable properly → 323753 - initialise meetOrSlice variable properly (checked in)
(Assignee)

Updated

12 years ago
Attachment #223311 - Attachment description: 327437 - createSVGAngle isn't implemented → 327437 - createSVGAngle isn't implemented (checked in)
(Assignee)

Updated

12 years ago
Attachment #223314 - Attachment description: 328137 - "ASSERTION: not a container" making one <svg:stop> a child of another< → 328137 - "ASSERTION: not a container" making one <svg:stop> a child of another (checked in)
(Assignee)

Updated

12 years ago
Attachment #223323 - Attachment description: 325728 - changes to markerWidth/markerHeight and orient not handled properly → 325728 - changes to markerWidth/markerHeight and orient not handled properly (checked in)
(In reply to comment #46)
> (In reply to comment #45)
> > It's a real implementation but it has a side effect that I eventually realised
> > was undesirable,
> 
> Is it undesirable in the sense that it causes problems (including problems
> handling unexpected things, which could be potential security problems), or
> just undesirable from a code maintainance perspective?
> 

As far as I know the fix does not have a security problem. 

It's probably not a big issue if you take this and don't take the other bigger fixes that rewrite it. On the branch it goes wrong in the same way that it goes wrong for text elements and that does not seem to have been something anyone has reported. 

After revisiting the code I don't think a user is affected by the element being transformable, the frame isn't so it should still draw the same.


(Assignee)

Updated

12 years ago
Attachment #223308 - Attachment description: 323669 - Percent attribute values in userSpace gradients don't work → 323669 - Percent attribute values in userSpace gradients don't work (checked in)
(Assignee)

Comment 48

12 years ago
Created attachment 227281 [details] [diff] [review]
implement SVGTSpanElement.getComputedTextLength

This version of SVGTSpanElement.getComputedTextLength does not change the parent class.
Attachment #223294 - Attachment is obsolete: true
Attachment #223294 - Flags: approval1.8.1?
(In reply to comment #48)
> Created an attachment (id=227281) [edit]
> implement SVGTSpanElement.getComputedTextLength
> 
> This version of SVGTSpanElement.getComputedTextLength does not change the
> parent class.
> 

This would address my concerns about inheritance, although for what its worth we removed all the NS_ASSERTION calls in this area under bug 339807.

You might also want to consider adding the IsEventName code to nsSVGTSpanElement.cpp similar to attachment (id=223300). Otherwise text and textPath will respond to events but tspan will not. The trunk has this fix already as part of bug 318597.

(Assignee)

Updated

12 years ago
Attachment #227281 - Flags: review?(tor)

Comment 50

12 years ago
Comment on attachment 227281 [details] [diff] [review]
implement SVGTSpanElement.getComputedTextLength

> NS_IMETHODIMP nsSVGTSpanElement::GetComputedTextLength(float *_retval)
> {
>-  NS_NOTYETIMPLEMENTED("nsSVGTSpanElement::GetComputedTextLength");
>-  return NS_ERROR_NOT_IMPLEMENTED;
>+//  nsIDOMSVGRect* bbox;
>+  nsCOMPtr<nsIDOMSVGRect> bbox;
>+
>+//  GetBBox(getter_AddRefs(bbox));

You can remove the commented lines.

>+  if (frame) {
>+    nsISVGChildFrame* svgframe;
>+    frame->QueryInterface(NS_GET_IID(nsISVGChildFrame),(void**)&svgframe);

While CallQueryInterface would be more appropriate, since this is essentially a clone of nsSVGGraphicsElement::GetBBox and that uses QI, we should probably leave it.

With the former change, r=tor.
Attachment #227281 - Flags: review?(tor) → review+
(Assignee)

Comment 51

12 years ago
Comment on attachment 227281 [details] [diff] [review]
implement SVGTSpanElement.getComputedTextLength

I realize this isn't blocking, but I believe this is the last SVG change we'll take for 1.8
Attachment #227281 - Flags: approval1.8.1?
Comment on attachment 227281 [details] [diff] [review]
implement SVGTSpanElement.getComputedTextLength

a=dbaron on behalf of drivers.  Please check in to MOZILLA_1_8_BRANCH and mark fixed1.8.1 once you have.
Attachment #227281 - Flags: approval1.8.1? → approval1.8.1+
(Assignee)

Comment 53

12 years ago
We're going to mark this bug done.

There are a few other individual issues we'll handle in the bugs.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.