410811 - Implement GetNearestViewportElement and GetFarthestViewportElement

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Robert Longson [:longsonr]]

Attachment: patch

As noted in bug 409376 comment 12

Comment 1

[Robert Longson [:longsonr]]

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

on second thoughts I'll move the implementation to nsSVGUtils then I only need one copy of each function.

Comment 2

[Robert Longson [:longsonr]]

Attachment: updated patch

same logic as previously but moved to nsSVGUtils so there is only one copy of each function

Comment 3

[tor]

Comment on attachment 295420 [details] [diff] [review]
updated patch

For a future bug/patch, it might be interested to see how these nsSVGUtil methods could be generalized to be used by other code in content/svg that does similar lookups.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

+      *aNearestViewportElement = SVGElement;
+      NS_ADDREF(*aNearestViewportElement);

use .swap()

+      nsCOMPtr<nsIDOMSVGElement> SVGElement = do_QueryInterface(element);
+      *aFarthestViewportElement = SVGElement;

hoist SVGElement to the outermost scope and use .swap() at the end of the function.

This looks OK but is this the right thing to be working on for 1.9?

Comment 5

[Robert Longson [:longsonr]]

Attachment: address sr comments

I'm doing this so that the litmus test for bug 409376 causes less confusion to testers see bug 409376 comment 12. At the moment when you look at the test and the reference image they are different and also you get errors in the error console. This patch corrects both issues so I'm hoping it will mean less work in the long run.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 296246 [details] [diff] [review]
address sr comments

OK, but we could create a testcase that doesn't have those problems.

Comment 7

[Robert Longson [:longsonr]]

Comment on attachment 296246 [details] [diff] [review]
address sr comments

Low risk. The new methods are only called by the SVG DOM so this implementation can't break any existing functionality.

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 296246 [details] [diff] [review]
address sr comments

a=beltzner for 1.9

Comment 9

[Robert Longson [:longsonr]]

checked in and in and in. Must try harder.

Comment 10

[Robert Longson [:longsonr]]

bug 483389 includes mochitests for this.