137382 - Redundant check for SVG namespace

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Håkan Waara]

There are two checks for the correct namespace in ConstructSVGFrame(), one can
be removed.

Comment 1

[Håkan Waara]

Attachment: Patch

bbaetz, can you r=?

Comment 2

[Håkan Waara]

Actually, since the only caller to ConstructSVGFrame has the namespace check as
well as a requirement, we can skip the other one in ConstructSVGFrame() too.

See
http://lxr.mozilla.org/seamonkey/source/layout/html/style/src/nsCSSFrameConstructor.cpp#7302

Comment 3

[Håkan Waara]

Attachment: Patch v2

Removes two redundant namespace checks.

Comment 4

[Håkan Waara]

Since this is a general (and trivial) CSSFrameConstructor patch, CC dbaron for r=.

BTW, I don't think sr= is required, since SVG is NPOB.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

You should get SVG module owner review.  Note that this re-enables
nsSVGAtoms::nameSpaceDeprecatedID.

Comment 6

[Håkan Waara]

Yes, I figured it was a bug that we didn't allow that namespace.

Comment 7

[Alex Fritze]

r=afri. 
Can you remove the test for the deprecated namespace as well please? We should 
only allow the official namespace.
At some stage we should go through the source with a fine comb and get rid of 
all references to nsSVGAtoms::nameSpaceDeprecatedID.

Comment 8

[Håkan Waara]

Attachment: Patch v3

Changes:

* Eliminate deprecated namespace.
* Fix some minor typos in nsIFrameManager.h
* Remove redundant namespace checks when constructing a SVG frame.

Afri, could you re-r= this?

Comment 9

[Alex Fritze]

Looks good.
r=afri

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

IMO, ConstructSVGFrame should at least have an assertion, since that code may be
restructured at some point in the near future.

Comment 11

[Håkan Waara]

Assert for what?

Comment 12

[Håkan Waara]

Oh, for being in the proper namespace?  I'll include that in the patch and then
checkin.

Comment 13

[Håkan Waara]

->moi

Comment 14

[Håkan Waara]

Checked in on the trunk today.

Comment 15

[Alex Fritze]

This patch introduces bogus assertions for xbl-wrapped svg elements - e.g. try 
http://www.croczilla.com/svg/xbl-shapes.xml.

The NS_ASSERTION in ConstructSVGFrame uses nsIContent::GetNameSpaceID(), which 
is incorrect for xbl-wrapped content. The namespace needs to be resolved using 
ResolveTag(). Also aNameSpaceID is already passed in as a parameter to 
ConstructSVGFrame(), so there is no need to actually re-resolve it.

Here's what the assertion should look like:

Index: nsCSSFrameConstructor.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp,v
retrieving revision 1.745
diff -u -r1.745 nsCSSFrameConstructor.cpp
--- nsCSSFrameConstructor.cpp	26 May 2002 17:07:07 -0000	1.745
+++ nsCSSFrameConstructor.cpp	14 Jun 2002 14:13:06 -0000
@@ -6986,8 +6988,7 @@
                                           nsIStyleContext*         
aStyleContext,
                                           nsFrameItems&            aFrameItems)
 {
-  NS_ASSERTION(NS_SUCCEEDED(aContent->GetNameSpaceID(aNameSpaceID)) && 
-            (aNameSpaceID == nsSVGAtoms::nameSpaceID), "SVG frame constructed 
in wrong namespace");
+  NS_ASSERTION(aNameSpaceID == nsSVGAtoms::nameSpaceID, "SVG frame constructed 
in wrong namespace");
 
   nsresult  rv = NS_OK;
   PRBool isAbsolutelyPositioned = PR_FALSE;

Comment 16

[Håkan Waara]

r=hwaara; go ahead and check-in the change.

Comment 17

[Alex Fritze]

Checked in as part of bug 182533.