Closed Bug 410811 Opened 12 years ago Closed 12 years ago
Nearest Viewport Element and Get Farthest Viewport Element
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.
same logic as previously but moved to nsSVGUtils so there is only one copy of each function
Attachment #295420 - Flags: review?(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.
Attachment #295420 - Flags: review?(tor) → review+
+ *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?
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 on attachment 296246 [details] [diff] [review] address sr comments OK, but we could create a testcase that doesn't have those problems.
Attachment #296246 - Flags: superreview?(roc) → superreview+
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.
Attachment #296246 - Flags: approval1.9?
Comment on attachment 296246 [details] [diff] [review] address sr comments a=beltzner for 1.9
Attachment #296246 - Flags: approval1.9? → approval1.9+
checked in and in and in. Must try harder.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
bug 483389 includes mochitests for this.
You need to log in before you can comment on or make changes to this bug.