Closed Bug 495296 Opened 11 years ago Closed 11 years ago
kill or de
COM ns ISVGText Content Metrics
When looking for "was hidden by" warnings, I found these: /home/swatinem/Coding/mozilla-central/layout/svg/base/src/nsSVGTextContainerFrame.h:72: warning: 'virtual nsresult nsSVGTextContainerFrame::GetNumberOfChars(PRInt32*)' was hidden /home/swatinem/Coding/mozilla-central/layout/svg/base/src/nsSVGTSpanFrame.h:88: warning: by 'virtual PRUint32 nsSVGTSpanFrame::GetNumberOfChars()' /home/swatinem/Coding/mozilla-central/layout/svg/base/src/nsSVGTextContainerFrame.h:73: warning: 'virtual nsresult nsSVGTextContainerFrame::GetComputedTextLength(float*)' was hidden /home/swatinem/Coding/mozilla-central/layout/svg/base/src/nsSVGTSpanFrame.h:89: warning: by 'virtual float nsSVGTSpanFrame::GetComputedTextLength()' /home/swatinem/Coding/mozilla-central/layout/svg/base/src/nsSVGTextContainerFrame.h:74: warning: 'virtual nsresult nsSVGTextContainerFrame::GetSubStringLength(PRUint32, PRUint32, float*)' was hidden /home/swatinem/Coding/mozilla-central/layout/svg/base/src/nsSVGTSpanFrame.h:90: warning: by 'virtual float nsSVGTSpanFrame::GetSubStringLength(PRUint32, PRUint32)' /home/swatinem/Coding/mozilla-central/layout/svg/base/src/nsSVGTextContainerFrame.h:79: warning: 'virtual nsresult nsSVGTextContainerFrame::GetCharNumAtPosition(nsIDOMSVGPoint*, PRInt32*)' was hidden /home/swatinem/Coding/mozilla-central/layout/svg/base/src/nsSVGTSpanFrame.h:91: warning: by 'virtual PRInt32 nsSVGTSpanFrame::GetCharNumAtPosition(nsIDOMSVGPoint*)' nsISVGTextContentMetrics uses COM-style outparameters, while other classes implement the same methods with non-COM signatures, leading to confusions and the above warning. nsISVGTextContentMetrics should be killed or at least updated to have COM-less methods. I will work up a patch as soon as I have time.
This turned out to be more complicated then I thought. There is only one Class implementing nsISVGTextContentMetrics, which is nsSVGTextContainerFrame. Some of nsISVGTextContentMetrics methods collide with those of nsISVGGlyphFragmentNode. nsSVGTSpanFrame implements the FragmentNode interface, forwarding the methods to TextContainerFrame. I got rid of nsISVGTextContentMetrics, using nsSVGTextContainerFrame instead and synching the method signatured with nsISVGGlyphFragmentNode. Apart from that I created a base class which implements nsIDOMSVGTextContentElement to get rid of copy pasta on the content side. The patch still has some issues, failing a few reftests. I haven't run mochitests yet. I will look into the failures soon.
Assignee: nobody → arpad.borsos
Status: NEW → ASSIGNED
The first patch missed some QueryFrame stuff. Passes reftest now. The failures I get on tryserver with this patch seem to be random/unrelated.
Comment on attachment 382125 [details] [diff] [review] patch [pushed: comment 5] Robert doesn't have to review this unless he wants to
I would have called the new file nsSVGTextContentElement rather than nsSVGTextContentElementBase as none of the other SVG base classes end in Base. (nsSVGElement, nsSVGStyleableElement etc). (In reply to comment #3) > Robert doesn't have to review this unless he wants to Phew. ;-)
Comment on attachment 382125 [details] [diff] [review] patch [pushed: comment 5] Pushed: http://hg.mozilla.org/mozilla-central/rev/d9090370b4dc I did remove the "Base" from the class/file name, as suggested by comment 4.
Attachment #382125 - Attachment description: patch → patch [pushed: comment 5]
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.