Closed Bug 216563 Opened 21 years ago Closed 20 years ago

conditional switch structure <switch> ignored.

Categories

(Core :: SVG, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: darryl, Assigned: scootermorris)

References

()

Details

Attachments

(4 files, 11 obsolete files)

4.80 KB, image/svg+xml
Details
24.85 KB, patch
Details | Diff | Splinter Review
876 bytes, patch
Details | Diff | Splinter Review
9.11 KB, patch
Details | Diff | Splinter Review
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5b) Gecko/20030809
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5b) Gecko/20030809

When there is a <switch> statement, all the children are rendered.
Only one should be rendered.

Note that this is a feature of SVG 1.1 full, basic, and tiny.

Reproducible: Always

Steps to Reproduce:
Load W2C svg tiny test suite test "struct-cond-010t.svg"
Actual Results:  
3 rectangles rendered, a red one, a green one, and a blue one.

Expected Results:  
Only the green rectangle should have rendered.
Only the green rectangle should render.
Switch handles children differently than normal.
The code for handling children is in nsCSSFrameConstructor.cpp
There is special case code for handling table children in nsCSSFrameConstructor.cpp
It seems to me that this would be the logical place to switch's handling of
children.
Logic for testing different conditionals only stubbed.
Just deals with switch's child processing.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment on attachment 134555 [details] [diff] [review]
patch for switch's processing of children

Patch looks good, but has unfortunately bit-rotted somewhat over the past 4
months. 
Darryl, do you have an updated version? 

Also:

>+ nsresult
>+ nsCSSFrameConstructor::TestConditions(nsIContent* aContent,
>+ 		                      PRBool&     aHasRequiredExtensions,
>+                                       PRBool&     aHasRequiredFeatures,
>+ 	                              PRBool&     aHasSystemLanguage)

There are some tabs here which need to be converted to spaces.


>Index: layout/html/style/src/nsCSSFrameConstructor.h
>===================================================================

Also needs untabbing.
Sorry, I haven't an up-to-date version of the patch.  
The SVG branch became unbuildable on Debian "testing" a couple of months,
at which point I stopped tracking things.  Pure laziness, Mea Culpae.
Attached patch Updated patch for <switch> (obsolete) — Splinter Review
Here is the updated patch for <switch>, which includes an implementation for
systemLanguage.  I'm not happy with the implementation (see comments about
DashMatchCompare), but it works.  Comments and suggestions welcome.
Comment on attachment 142979 [details] [diff] [review]
Updated patch for <switch>

r=afri with the following changes:

TestConditions and SwitchProcessChildren should be renamed to something like
TestSVGConditions and SVGSwitchProcessChildren.

>Index: layout/html/style/src/nsCSSFrameConstructor.cpp
>===================================================================
>+ 
>+ // NeedSpecialFrameReframe uses this until we decide what to do about IsInlineFrame() above

This comment shouldn't be there.

>+   if (NS_OK == rv) {
>+     // For now, claim that mozilla's SVG implimentation supports

Should be "implementation", not "implimentation".

>+     // For now, claim that mozilla's SVG implimentation supports
>+     // all features of SVG...  This is, of course, a horrific lie.
>+     // TODO: test against actual implimented features of SVG.

"implementation", "implemented"

>+     NS_WARNING("requiredFeatures conditional test not implimented");

"implemented"
Attachment #142979 - Flags: superreview?(bzbarsky)
Attachment #142979 - Flags: review+
Comment on attachment 142979 [details] [diff] [review]
Updated patch for <switch>

For future reference, please use more context and the -p option to diff when
making the patch.  Also, make unified diffs.  A patch made with -pu8 is much
easier to review than this one....

>Index: layout/html/style/src/nsCSSFrameConstructor.cpp

>+ #include "unicharutil/nsUnicharUtils.h" // to get 

You should not need the "unicharutil/" part.  Add the module to REQUIRES in the
Makefile.in if necessary.

>+ #ifdef MOZ_SVG
>+ static PRBool gGotLangPrefs = PR_FALSE;
>+ static nsXPIDLCString gLangPrefs;
>+ #endif

I'm not sure that static string objects are completely kosher... darin or
dbaron would be the ones to ask.

>+ // This is taken directly from content/html/style/src/nsCSSStyleSheet.cpp,
>+ // which strongly suggests that it should be a public function somewhere - intl?

Not intl, by any means.  Perhaps nsStyleUtils or something.... (which we may
want to create).  I suppose it's OK to leave this as-is for now, but please
file a follup bug on this issue.

>+ nsCSSFrameConstructor::TestConditions(nsIContent* aContent,
>+   rv = aContent->GetAttr(kNameSpaceID_None, nsSVGAtoms::requiredExtensions, value);
>+ 
>+   if (NS_OK == rv) {

Surely you mean NS_CONTENT_ATTR_HAS_VALUE?

>+     aHasRequiredExtensions = PR_FALSE;
>+   } else if (NS_CONTENT_ATTR_NOT_THERE == rv) {
>+     aHasRequiredExtensions = PR_TRUE;
>+     rv = NS_OK;
>+   }

What about NS_CONTENT_ATTR_NO_VALUE?

Perhaps you really want:

  aHasRequiredExtensions = (rv != NS_CONTENT_ATTR_HAS_VALUE);

for now?

>+   rv = aContent->GetAttr(kNameSpaceID_None, nsSVGAtoms::requiredFeatures, value);

Similar issues with rv here.

> +   rv = aContent->GetAttr(kNameSpaceID_None, nsSVGAtoms::systemLanguage, value);

And here.

> +         prefBranch->GetCharPref( "intl.accept_languages", getter_Copies(gLangPrefs) );

This code fails to deal with changes to the pref without restarting....

Also, that pref is not a char pref.  It's a nsIPrefLocalizedString complex
value pref.

>+       nsString langprefs = NS_ConvertASCIItoUCS2(gLangPrefs);

This should be

  NS_ConvertASCIItoUCS2 langPrefs(gLangPrefs);

Though really, why not just store gLangPrefs in UCS2 so that you don't have to
deal with this conversion and whitespace-stripping every time?

>+ nsCSSFrameConstructor::SwitchProcessChildren(nsIPresShell*            aPresShell,
>+                                              nsIPresContext*          aPresContext,
>+                                              nsFrameConstructorState& aState,
>+                                              nsIContent*              aContent,
>+                                              nsIFrame*                aFrame,
>+                                              PRBool                   aCanHaveGeneratedContent,
>+                                              nsFrameItems&            aFrameItems,
>+                                              PRBool                   aParentIsBlock,
>+                                              nsTableCreator*          aTableCreator)

Most of these args are unused (which is a bug, imo).  For example, you're
probably messing up table things when aTableCreator is non-null here (though
I'm not quite sure whether that situation arises).  You're also not creating
generated content on the <switch>; is that correct?  If so, you don't need the
aCanHaveGeneratedContent boolean, do you?

>+   PRBool isFinished = PR_FALSE;

No need for this.

>+     rv = TestConditions(child,
>+                         hasRequiredExtensions,
>+                         hasRequiredFeatures,
>+                         hasSystemLanguage);
>+ 
>+     if (NS_FAILED(rv))
>+       return rv;
>+ 
>+     if (hasRequiredExtensions &&
>+         hasRequiredFeatures &&
>+         hasSystemLanguage) {

So I'm trying to understand the spec here.  If we have something like:

<switch>
aaa <bbb />
</switch>

should the <bbb /> be rendered?  If I read the spec right, the answer is "no". 
Then again, this is not a valid SVG file per spec, again if I read it right. 
But we're not a validating parser...

The point is, in my opinion TestConditions should either return false for all
non-element content nodes for all three booleans or we should just skip over
them in this code.

>+       if (child->IsContentOfType(nsIContent::eELEMENT)) {
>+         isFinished = PR_TRUE;
>+       }

just use 'break'

>+       if (NS_FAILED(rv))
>+         return rv;

And put this before that check, of course.
Attachment #142979 - Flags: superreview?(bzbarsky) → superreview-
Thanks for the userful comments.  I'm just getting started in hacking mozilla
and this was quite helpful.  I've incorporated most of these comments, but I'm
not sure how to implement the comment about non-element content nodes, but I'll
try to ponder my way through.  In the meantime, before resubmitting this patch,
I would like to implement the requiredFeatures test. The code itself seems
pretty straightforward (convert a string to a bitmask and then compare that
bitmask to the current "feature set".  My question is where should I put the
tables necessary to implement this?  My initial guess would be in a new .h file
in content/shared/public (nsSVGFeatures.h)?  At any rate, as soon as I have the
features stuff implemented, I'll submit an updated patch so that the entire
feature can be completed.
Scooter: Since you've now taken over development of this, you should assign the
bug to yourself.

(In reply to comment #9)
> I would like to implement the requiredFeatures test. The code itself seems
> pretty straightforward (convert a string to a bitmask and then compare that
> bitmask to the current "feature set".  My question is where should I put the
> tables necessary to implement this?  My initial guess would be in a new .h file
> in content/shared/public (nsSVGFeatures.h)?  At any rate, as soon as I have the
> features stuff implemented, I'll submit an updated patch so that the entire
> feature can be completed.

According to the 1.1 SVG specs (http://www.w3.org/TR/SVG11/feature.html), the
requiredFeatures test is broadly equivalent to DOMImplementation::hasFeature(). 
We have an implementation for the latter in Mozilla already. It is based on
nsGenericElement::InternalIsSupported(). I haven't looked at the code in detail,
but we should probably hook into this mechanism to support both requiredFeatures
and hasFeature() at the same time. Note also that potentially we might want to
determine supported features dynamically, based on the current rendering engine
(although this is probably overkill).
(In reply to comment #10)
> Scooter: Since you've now taken over development of this, you should assign the
> bug to yourself.
> 
OK, but I don't seem to have the capability to change either the bug status (NEW
to ASSIGNED) or the assignee.
> 
> According to the 1.1 SVG specs (http://www.w3.org/TR/SVG11/feature.html), the
> requiredFeatures test is broadly equivalent to DOMImplementation::hasFeature(). 
> We have an implementation for the latter in Mozilla already. It is based on
> nsGenericElement::InternalIsSupported(). I haven't looked at the code in detail,
> but we should probably hook into this mechanism to support both requiredFeatures
> and hasFeature() at the same time. Note also that potentially we might want to
> determine supported features dynamically, based on the current rendering engine
> (although this is probably overkill).

OK, the feature stuff is complete, and I have hooked it in to
nsGenericElement::InternalIsSupported, although I don't have any test cases that
I can use to test it.  Does anyone have any code that uses the DOM to call
hasFeature()?  If so, I will adapt it to SVG.

The next challenge is to define the current feature string.  Here is my guess so
far (emphasis on guess):

SVG_F_CORE              Yes
SVG_F_STRUCTURE         Yes
SVG_F_CONTAINER         Yes
SVG_F_CONDITIONAL       Yes
SVG_F_IMAGE             No
SVG_F_STYLE             Yes
SVG_F_VIEWPORT          Yes?
SVG_F_SHAPE             Yes
SVG_F_TEXT              Yes // Still missing features
SVG_F_PAINTATTR         Yes
SVG_F_OPACITYATTR       Yes // Doesn't work for groups -- inheritance problem?
SVG_F_GRAPHICSATTR      Yes
SVG_F_MARKER            Yes
SVG_F_COLORPROFILE      No
SVG_F_GRADIENT          No
SVG_F_PATTERN           Yes
SVG_F_CLIP              No
SVG_F_MASK              No
SVG_F_FILTER            No
SVG_F_BASICSTRUCTURE    Yes
SVG_F_BASICTEXT         Yes
SVG_F_BASICPAINTATTR    Yes
SVG_F_BASICGRAPHICSATTR Yes
SVG_F_BASICCLIP         No
SVG_F_BASICFILTER       No
SVG_F_BASICFONT         No?
SVG_F_DOCUMENTEVENTS    No
SVG_F_GRAPHICALEVENTS   No
SVG_F_ANIMATIONEVENTS   No
SVG_F_CURSOR            Yes
SVG_F_HYPERLINKING      Yes
SVG_F_XLINK             No
SVG_F_EXTERNALRESOURCES No
SVG_F_VIEW              No
SVG_F_SCRIPT            No
SVG_F_FONT              No
SVG_F_EXTENSIBILITY     No
SVG_F_ANIMATION         No


Reassigning to Scooter
Assignee: alex → scootermorris
Status: NEW → ASSIGNED
Attached patch Patch version 2 (obsolete) — Splinter Review
OK, here is another patch that implements <switch> including systemLanguage and
requiredFeatures.  The features are currently defined as a long long bitmask,
which turned out to be sort of messy.  A future direction would be to
encapsulate all of the Feature stuff into its own class and provide a method to
register a new feature.  This could also be used to implement
requiredExtensions.  I'm open to guidance on that.  Also note that currently
all of the feature and system language code is in nsSVGStyleUtils.cpp in the
layout tree.  Is this the right place for it?  Its called from
nsCSSFrameConstructor.cpp, but its also called from nsGenericElement.cpp in the
content tree for the DOM hookup.
Attachment #142979 - Attachment is obsolete: true
> Index: layout/html/style/src/Makefile.in
> ===================================================================
>  CPPSRCS		= \
>  		nsCSSFrameConstructor.cpp \
>  		nsCSSRendering.cpp \
>  		nsCSSColorUtils.cpp \
>  		nsFrameContentIterator.cpp \
>  		nsChildIterator.cpp \
> +		nsSVGStyleUtils.cpp \
>  		$(NULL)

You should only include nsSVGStyleUtils.cpp for SVG builds, but I
think html/style/src/ is not the right place for this anyway.

I would move DashMatchCompare() and the language feature test back to
nsCSSFrameConstructor.cpp (because it is not really SVG-specific and
begs to be consolidated with the other DashMatchCompare function) and
rename nsSVGStyleUtils.{cpp,h} to nsSVGFeatures.{cpp,h} and move these
files to content/svg/content/src/.

> Index: layout/html/style/src/nsCSSFrameConstructor.cpp
> ===================================================================
> ... 
> +// NeedSpecialFrameReframe uses this until we decide what to do about
IsInlineFrame() above
> +
> ... 
>  
> -	   // TITLEBAR CONSTRUCTION
> -	   else if (aTag == nsXULAtoms::titlebar) {
> +     // TITLEBAR CONSTRUCTION
> +     else if (aTag == nsXULAtoms::titlebar) {
>          processChildren = PR_TRUE;
> ... 
> -		  // Boxes can scroll.
> +      // Boxes can scroll.
> ...
>  
> -	   // RESIZER CONSTRUCTION
> -	   else if (aTag == nsXULAtoms::resizer) {
> +     // RESIZER CONSTRUCTION
> +     else if (aTag == nsXULAtoms::resizer) {
> ...
>    switch (aDisplay->mOverflow) {
> -  	case NS_STYLE_OVERFLOW_SCROLL:
> -  	case NS_STYLE_OVERFLOW_AUTO:
> -  	case NS_STYLE_OVERFLOW_SCROLLBARS_NONE:
> -  	case NS_STYLE_OVERFLOW_SCROLLBARS_HORIZONTAL:
> -  	case NS_STYLE_OVERFLOW_SCROLLBARS_VERTICAL:
> -	    return PR_TRUE;
> +    case NS_STYLE_OVERFLOW_SCROLL:
> +    case NS_STYLE_OVERFLOW_AUTO:
> +    case NS_STYLE_OVERFLOW_SCROLLBARS_NONE:
> +    case NS_STYLE_OVERFLOW_SCROLLBARS_HORIZONTAL:
> +    case NS_STYLE_OVERFLOW_SCROLLBARS_VERTICAL:
> +      return PR_TRUE;

This stuff shouldn't be in the patch.


> @@ -6717,8 +6721,17 @@ nsCSSFrameConstructor::ConstructMathMLFr
>      }
>    }
>    return rv;
>  }
>  #endif // MOZ_MATHML
>  
>  // SVG 
>  #ifdef MOZ_SVG
> +
> +nsresult
> +nsCSSFrameConstructor::TestSVGConditions(nsIContent* aContent,
> +                                      PRBool&     aHasRequiredExtensions,
> +                                      PRBool&     aHasRequiredFeatures,
> +                                      PRBool&     aHasSystemLanguage)
> +{
> +  nsresult rv = NS_OK;
> +  nsAutoString value;  // Should be nsAutoString, right??

Yes, so you can remove the comment ;-)

> +  // Required Features
> +  rv = aContent->GetAttr(kNameSpaceID_None, nsSVGAtoms::requiredFeatures, value);
> +  if (NS_CONTENT_ATTR_HAS_VALUE == rv) {
> +    aHasRequiredFeatures = NS_SVG_TestFeature(value);

requiredFeatures is a whitespace-separated list of features, so you
need to loop over each one here (or have a function that tests for a
list of features, not a single feature).


> +    // Get our language preferences
> +    nsXPIDLCString gLangPrefs;

Now that this isn't global anymore it shouldn't be prefixed with a 'g'

> +    nsCOMPtr<nsIPrefBranch> prefBranch(do_GetService(NS_PREFSERVICE_CONTRACTID));
> +    if (prefBranch) {
> +      prefBranch->GetCharPref("intl.accept_languages", 
> +                              getter_Copies(gLangPrefs) );
> +    }


I think we might actually need something like the following here:

    nsAutoString langPrefs;
    nsCOMPtr<nsIPrefLocalizedString> prefString;
    prefBranch->GetComplexValue("intl.accept_languages",
                                NS_GET_IID(nsIPrefLocalizedString),
                                getter_AddRefs(prefString));
    if (prefString) {
      prefString->ToString(getter_Copies(langPrefs));
    }

Boris made a comment to that effect. Not sure whether GetCharPref()
works. nsDocument::RetrieveRelevantHeaders uses it... 


> Index: content/base/src/nsGenericElement.cpp
> +
> +#ifdef	MOZ_SVG
> +  // Check for SVG Features
> +  // This will *not* work if SVG is dynamically loaded
> +  else if (NS_SVG_TestFeature(aFeature)) {
> +    *aReturn = PR_TRUE;
> +  }
> +#endif /* MOZ_SVG */
> +  

Don't we need to check against the version as well (1.1)?

 
> --- layout/nnn/style/src/nsSVGFeaturesList.h	1969-12-31 16:00:00.000000000 -0800
> +++ layout/html/style/src/nsSVGFeaturesList.h	2004-03-19 17:36:42.000000000 -0800

There's an "Index:XXX"-line missing here, hence the patch behaves a
bit strangely when view through bugzilla diff.
Also, I'd move this to content/svg/content/src/.

> +// Features (SVG 1.1 style)
> +
> +// Static festures
> +SVG_FEATURE(SVG_F_CORE, 0, "http://www.w3.org/TR/SVG11/feature#CoreAttribute")
> +SVG_FEATURE(SVG_F_STRUCTURE, 0, "http://www.w3.org/TR/SVG11/feature#Structure")
> +SVG_FEATURE(SVG_F_CONTAINER, 0,
"http://www.w3.org/TR/SVG11/feature#ContainerAttribute")
> +SVG_FEATURE(SVG_F_CONDITIONAL, 0,
"http://www.w3.org/TR/SVG11/feature#ConditionalProcessing")
> +SVG_FEATURE(SVG_F_IMAGE, 0, "http://www.w3.org/TR/SVG11/feature#Image")
...

How about restructuring this to not build a table and bitmask, but to
generate the feature tests directly:

SVG_SUPPORTED_FEATURE("http://www.w3.org/TR/SVG11/feature#CoreAttribute")
SVG_SUPPORTED_FEATURE(""http://www.w3.org/TR/SVG11/feature#Structure")
SVG_SUPPORTED_FEATURE("http://www.w3.org/TR/SVG11/feature#ContainerAttribute")
SVG_SUPPORTED_FEATURE("http://www.w3.org/TR/SVG11/feature#ConditionalProcessing")
SVG_UNSUPPORTED_FEATURE("http://www.w3.org/TR/SVG11/feature#Image")
...

As far as I can see these defintions coupled with the code below
should be enough to implement the tests.

> --- layout/nnn/style/src/nsSVGStyleUtils.h	1969-12-31 16:00:00.000000000 -0800
> +++ layout/html/style/src/nsSVGStyleUtils.h	2004-03-19 17:35:01.000000000 -0800

Index-line missing. Also this should be moved to
content/svg/content/src/nsSVGFeatures.h

> +// Define the bit masks for each of the features
> +
> +// Static
> +#define SVG_F_CORE              0x00000001
> +#define SVG_F_STRUCTURE         0x00000002
> +#define SVG_F_CONTAINER         0x00000004
> +#define SVG_F_CONDITIONAL       0x00000008
> ...
> +// Definition for the feature table struct
> +struct SVG_FeatureTable {
> +  PRUint32  fbitsl;
> +  PRUint32  fbitsh;
> +  const char *fstr;
> +};

Not needed anymore

> --- layout/nnn/style/src/nsSVGStyleUtils.cpp	1969-12-31 16:00:00.000000000 -0800
> +++ layout/html/style/src/nsSVGStyleUtils.cpp	2004-03-19 17:32:35.000000000 -0800

Index-line missing & should be moved to
content/svg/content/src/nsSVGFeatures.cpp

Instead of this: 
> +static const SVG_FeatureTable NS_SVG_FeatureTable[] = {
> +#define SVG_FEATURE(bitsl_, bitsh_, string_) {bitsl_ , bitsh_ , string_ },
> +#include "nsSVGFeaturesList.h"
> +#undef SVG_FEATURE
> +  { 0, 0, nsnull }
> +};
> ...
> +PRBool
> +NS_SVG_TestFeature(const nsAString& fstr) {
> +  PRUint64 fbits = NS_SVG_String2Feature(fstr);
> +  PRUint64 dom = myLL_II2L(SVG_F_DOM,0);
> +  PRUint64 ndom;
> +  PRUint64 a;
> ...

... how about something like this:

PRBool NS_SVG_TestFeature(const nsAString& fstr) {
#define SVG_SUPPORTED_FEATURE(str) if (fstr.equals(str)) return PR_TRUE;
#include "nsSVGFeaturesList.h"
#undef SVG_SUPPORTED_FEATURE
  return PR_FALSE;
}
(In reply to comment #14)

> You should only include nsSVGStyleUtils.cpp for SVG builds, but I
> think html/style/src/ is not the right place for this anyway.
> 
> I would move DashMatchCompare() and the language feature test back to
> nsCSSFrameConstructor.cpp (because it is not really SVG-specific and
> begs to be consolidated with the other DashMatchCompare function) and
> rename nsSVGStyleUtils.{cpp,h} to nsSVGFeatures.{cpp,h} and move these
> files to content/svg/content/src/.

OK, Will do.

> 
> > Index: layout/html/style/src/nsCSSFrameConstructor.cpp
> > ===================================================================
> > ... 

> 
> This stuff shouldn't be in the patch.

I know -- the tabbing was messed up in this section, so I fixed it.  I'll edit
it out of the patch update.
> 
> 
> > @@ -6717,8 +6721,17 @@ nsCSSFrameConstructor::ConstructMathMLFr
> >      }
> >    }
> >    return rv;
> >  }
> >  #endif // MOZ_MATHML
> >  
> >  // SVG 
> >  #ifdef MOZ_SVG
> > +
> > +nsresult
> > +nsCSSFrameConstructor::TestSVGConditions(nsIContent* aContent,
> > +                                      PRBool&     aHasRequiredExtensions,
> > +                                      PRBool&     aHasRequiredFeatures,
> > +                                      PRBool&     aHasSystemLanguage)
> > +{
> > +  nsresult rv = NS_OK;
> > +  nsAutoString value;  // Should be nsAutoString, right??
> 
> Yes, so you can remove the comment ;-)

Alright, alright....done. :-)

> 
> > +  // Required Features
> > +  rv = aContent->GetAttr(kNameSpaceID_None, nsSVGAtoms::requiredFeatures,
value);
> > +  if (NS_CONTENT_ATTR_HAS_VALUE == rv) {
> > +    aHasRequiredFeatures = NS_SVG_TestFeature(value);
> 
> requiredFeatures is a whitespace-separated list of features, so you
> need to loop over each one here (or have a function that tests for a
> list of features, not a single feature).

Bummer!  Nice catch.  It will now call NS_SVG_TestFeatures...

> 
> 
> > +    // Get our language preferences
> > +    nsXPIDLCString gLangPrefs;
> 
> Now that this isn't global anymore it shouldn't be prefixed with a 'g'

Okey dokey...

> 
> > +    nsCOMPtr<nsIPrefBranch>
prefBranch(do_GetService(NS_PREFSERVICE_CONTRACTID));
> > +    if (prefBranch) {
> > +      prefBranch->GetCharPref("intl.accept_languages", 
> > +                              getter_Copies(gLangPrefs) );
> > +    }
> 
> 
> I think we might actually need something like the following here:
> 
>     nsAutoString langPrefs;
>     nsCOMPtr<nsIPrefLocalizedString> prefString;
>     prefBranch->GetComplexValue("intl.accept_languages",
>                                 NS_GET_IID(nsIPrefLocalizedString),
>                                 getter_AddRefs(prefString));
>     if (prefString) {
>       prefString->ToString(getter_Copies(langPrefs));
>     }
> 
> Boris made a comment to that effect. Not sure whether GetCharPref()
> works. nsDocument::RetrieveRelevantHeaders uses it... 

I saw Boris' comment, but the only other place in the code that I could find
that referenced and used the intl.accept_languages stuff treated it as a string
pref, and I wasn't sure how to handle complexvalue prefs, so I left it be.  I'll
use your code and make it work.

> 
> 
> > Index: content/base/src/nsGenericElement.cpp
> > +
> > +#ifdef	MOZ_SVG
> > +  // Check for SVG Features
> > +  // This will *not* work if SVG is dynamically loaded
> > +  else if (NS_SVG_TestFeature(aFeature)) {
> > +    *aReturn = PR_TRUE;
> > +  }
> > +#endif /* MOZ_SVG */
> > +  
> 
> Don't we need to check against the version as well (1.1)?

Probably should, but its redundant since in SVG 1.1 the feature strings
themselves include the version, and we're required to support the old style
anyways.  I'll add the version check.

> 
>  
> > --- layout/nnn/style/src/nsSVGFeaturesList.h	1969-12-31 16:00:00.000000000 -0800
> > +++ layout/html/style/src/nsSVGFeaturesList.h	2004-03-19 17:36:42.000000000
-0800
> 
> There's an "Index:XXX"-line missing here, hence the patch behaves a
> bit strangely when view through bugzilla diff.
> Also, I'd move this to content/svg/content/src/.
> 

> 
> How about restructuring this to not build a table and bitmask, but to
> generate the feature tests directly:
> 
> SVG_SUPPORTED_FEATURE("http://www.w3.org/TR/SVG11/feature#CoreAttribute")
> SVG_SUPPORTED_FEATURE(""http://www.w3.org/TR/SVG11/feature#Structure")
> SVG_SUPPORTED_FEATURE("http://www.w3.org/TR/SVG11/feature#ContainerAttribute")
> SVG_SUPPORTED_FEATURE("http://www.w3.org/TR/SVG11/feature#ConditionalProcessing")
> SVG_UNSUPPORTED_FEATURE("http://www.w3.org/TR/SVG11/feature#Image")
> ...
> 
> As far as I can see these defintions coupled with the code below
> should be enough to implement the tests.
> 
> ... how about something like this:
> 
> PRBool NS_SVG_TestFeature(const nsAString& fstr) {
> #define SVG_SUPPORTED_FEATURE(str) if (fstr.equals(str)) return PR_TRUE;
> #include "nsSVGFeaturesList.h"
> #undef SVG_SUPPORTED_FEATURE
>   return PR_FALSE;
> }
> 

Very nice -- this will remove all of that 64-bit junk, although the cost is a
potentially expensive if cascade.

At any rate, look for a new patch later this week.  I'll also try to figure out
why I lost the index lines.... I'm sure it has to do with how I generated the
diffs for new files.

Thanks for the comments!

Attached patch Patch version 3 (obsolete) — Splinter Review
OK, here is the latest installment.  I added the capability to handle multiple
features (whitespace delimited) and moved the files around as suggested.  I did
not implement the version checking in the DOM interface because doing so would
break the W3C conformance test (they are testing for version 2.0?).  Everything
else seems to work OK.
Attachment #144373 - Attachment is obsolete: true
Comment on attachment 144672 [details] [diff] [review]
Patch version 3

> // SVG - rods
> #ifdef MOZ_SVG
>+
>+  nsresult TestSVGConditions(nsIContent* aContent,
>+                          PRBool&     aHasRequiredExtensions,
>+                          PRBool&     aHasRequiredFeatures,
>+                          PRBool&     aHasSystemLanguage);
>+
>+  nsresult SVGSwitchProcessChildren(nsIPresShell*            aPresShell,
>+                                 nsIPresContext*          aPresContext,
>+                                 nsFrameConstructorState& aState,
>+                                 nsIContent*              aContent,
>+                                 nsIFrame*                aFrame,
>+                                 nsFrameItems&            aFrameItems);
>+

Can you fix the indentations here, please?

>   nsresult ConstructSVGFrame(nsIPresShell*               aPresShell,
>                                 nsIPresContext*          aPresContext,
>                                 nsFrameConstructorState& aState,
>                                 nsIContent*              aContent,
>                                 nsIFrame*                aParentFrame,
>                                 nsIAtom*                 aTag,
>                                 PRInt32                  aNameSpaceID,
>                                 nsStyleContext*          aStyleContext,

And for this as well?

>+  // This will *not* work if SVG is dynamically loaded

Please remove this comment; SVG is always statically compiled in if MOZ_SVG is
defined.

>+  else if (NS_SVG_TestFeature(aFeature)) {
>+    // Should we test the version also?  The SVG Test
>+    // Suite (struct-dom-02-b.svg) uses version 2.0 in the
>+    // domImpl.hasFeature call. This makes no sense to me,
>+    // so I'm going to skip testing the version here for now....

Yeah, it doesn't make any sense to me either, so not checking is probably the
pragmatic thing...

>Index: content/svg/content/src/nsSVGFeaturesList.h
>===================================================================
>--- nnn/svg/content/src/nsSVGFeaturesList.h	1969-12-31 16:00:00.000000000 -0800
>+++ content/svg/content/src/nsSVGFeaturesList.h	2004-03-23 19:52:39.000000000 -0800
>@@ -0,0 +1,110 @@
>+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
>+/* ***** BEGIN LICENSE BLOCK *****
>+ * Version: NPL 1.1/GPL 2.0/LGPL 2.1
>+ *
>+ * The contents of this file are subject to the Netscape Public License
>+ * Version 1.1 (the "License"); you may not use this file except in
>+ * compliance with the License. You may obtain a copy of the License at
>+ * http://www.mozilla.org/NPL/
>+ *
>+ * Software distributed under the License is distributed on an "AS IS" basis,
>+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
>+ * for the specific language governing rights and limitations under the
>+ * License.
>+ *
>+ * The Original Code is mozilla.org code.
>+ *
>+ * The Initial Developer of the Original Code is 
>+ * Netscape Communications Corporation.
>+ * Portions created by the Initial Developer are Copyright (C) 1998
>+ * the Initial Developer. All Rights Reserved.
>+ *
>+ * Original Author: Scooter Morris (scootermorris@comcast.net)
>+ *
>+ * Contributor(s):
>+ *
>+ *
>+ * Alternatively, the contents of this file may be used under the terms of
>+ * either the GNU General Public License Version 2 or later (the "GPL"), or
>+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
>+ * in which case the provisions of the GPL or the LGPL are applicable instead
>+ * of those above. If you wish to allow use of your version of this file only
>+ * under the terms of either the GPL or the LGPL, and not to allow others to
>+ * use your version of this file under the terms of the NPL, indicate your
>+ * decision by deleting the provisions above and replace them with the notice
>+ * and other provisions required by the GPL or the LGPL. If you do not delete
>+ * the provisions above, a recipient may use your version of this file under
>+ * the terms of any one of the NPL, the GPL or the LGPL.
>+ *
>+ * ***** END LICENSE BLOCK ***** */

This is an NPL license header. Unless you work for Netscape you'll probably
want to change this for an MPL license header. 

>Index: content/svg/content/src/nsSVGFeatures.cpp
>===================================================================
>--- nnn/svg/content/src/nsSVGFeatures.cpp	1969-12-31 16:00:00.000000000 -0800
>+++ content/svg/content/src/nsSVGFeatures.cpp	2004-03-23 21:42:10.000000000 -0800
>@@ -0,0 +1,84 @@
>+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
>+// vim:cindent:ts=2:et:sw=2:
>+/* ***** BEGIN LICENSE BLOCK *****
>+ * Version: NPL 1.1/GPL 2.0/LGPL 2.1
[...]
>+ * ***** END LICENSE BLOCK ***** */

Should also be MPL rather than NPL.

>+
>+#ifdef  MOZ_SVG

Not needed, since the file is only compiled for SVG builds anyway.

>+#endif /* MOZ_SVG */

ditto

r=afri with these changes.
Attachment #144672 - Flags: review+
Attached patch Updated patch 3 (obsolete) — Splinter Review
Patch 3 with Alex's changes.  Ready for sr?
Attachment #144672 - Attachment is obsolete: true
Attachment #145017 - Flags: superreview?
Attachment #145017 - Flags: superreview? → superreview?(dbaron)
Comment on attachment 145017 [details] [diff] [review]
Updated patch 3

>+#ifdef  MOZ_SVG
>+// This is taken directly from content/html/style/src/nsCSSStyleSheet.cpp,
>+static PRBool
>+DashMatchCompare(const nsAString& aAttributeValue,

Don't copy code like this.  Move it to nsStyleUtil or nsContentUtils (probably
the former, but maybe someone else disagrees).

>+// Test to see if this language is supported
>+static PRBool
>+SVG_TestLanguage(const nsAString& lstr, const nsAString& prefs) {

opening brace of functions on a new line, to be consistent with surrounding
code

Also, it's worth commenting that the function is O(M*N), since that could
become a problem at some point.

>+  // NS_ConvertASCIItoUCS2 langprefs(pref);

what's this doing here?

>+  nsAutoString langprefs(prefs);
>+  langprefs.StripWhitespace();

Is |lstr| guaranteed to have the whitespace already removed?  If so, comment,
and perhaps assert.  If not, should you remove it?

>+// NeedSpecialFrameReframe uses this until we decide what to do about IsInlineFrame() above
>+

What's this comment doing here?  This seems like a merge error.

>+nsresult
>+nsCSSFrameConstructor::TestSVGConditions(nsIContent* aContent,
>+                                         PRBool&     aHasRequiredExtensions,
>+                                         PRBool&     aHasRequiredFeatures,
>+                                         PRBool&     aHasSystemLanguage)
>+{
>+  nsresult rv = NS_OK;
>+  nsAutoString value;
>+
>+  // Only elements can have tests on them
>+  if (! aContent->IsContentOfType(nsIContent::eELEMENT)) {
>+    aHasRequiredExtensions = PR_FALSE;
>+    aHasRequiredFeatures = PR_FALSE;
>+    aHasSystemLanguage = PR_FALSE;
>+    return rv;
>+  }
>+
>+  // Required Extensions
>+  //
>+  // The requiredExtensions  attribute defines a list of required language
>+  // extensions. Language extensions are capabilities within a user agent that
>+  // go beyond the feature set defined in the SVG specification.
>+  // Each extension is identified by a URI reference.
>+  rv = aContent->GetAttr(kNameSpaceID_None, nsSVGAtoms::requiredExtensions, value);
>+
>+  if (NS_CONTENT_ATTR_HAS_VALUE == rv) {
>+    // For now, claim that mozilla's SVG implementation supports
>+    // no extensions.  So, if extensions are required, we don't have
>+    // them available.
>+    aHasRequiredExtensions = PR_FALSE;
>+  } else if ((NS_CONTENT_ATTR_NOT_THERE == rv) || (NS_CONTENT_ATTR_NO_VALUE == rv)) {
>+    aHasRequiredExtensions = PR_TRUE;
>+    rv = NS_OK;
>+  } 
>+  if (NS_FAILED(rv))
>+    return rv;

NS_CONTENT_ATTR_{NOT_THERE,HAS_VALUE,NO_VALUE} are all success values, so you
can check rv and return before the other code, and skip the rv = NS_OK.

Having an |else if| rather than just an |else| could potentially leave
aHasRequiredExtensions uninitialized in some success cases, which you shouldn't
do.  If you really want to check that it's one of those two values, use
NS_ASSERTION, not |if|.

>+  // Required Features
>+  rv = aContent->GetAttr(kNameSpaceID_None, nsSVGAtoms::requiredFeatures, value);
>+  if (NS_CONTENT_ATTR_HAS_VALUE == rv) {
>+    aHasRequiredFeatures = NS_SVG_TestFeatures(value);
>+  } else if ((NS_CONTENT_ATTR_NOT_THERE == rv) || (NS_CONTENT_ATTR_NO_VALUE == rv)) {
>+    aHasRequiredFeatures = PR_TRUE;
>+    rv = NS_OK;
>+  }
>+  if (NS_FAILED(rv))
>+    return rv;

Same here.

>+  // systemLanguage
>+  //
>+  // Evaluates to "true" if one of the languages indicated by user preferences
>+  // exactly equals one of the languages given in the value of this parameter,
>+  // or if one of the languages indicated by user preferences exactly equals a
>+  // prefix of one of the languages given in the value of this parameter such
>+  // that the first tag character following the prefix is "-".
>+  rv = aContent->GetAttr(kNameSpaceID_None, nsSVGAtoms::systemLanguage, value);
>+  if (NS_CONTENT_ATTR_HAS_VALUE == rv) {
>+    // Get our language preferences
>+    nsXPIDLString langPrefs;
>+
>+    nsCOMPtr<nsIPrefLocalizedString> prefString;
>+    nsCOMPtr<nsIPrefBranch> prefBranch(do_GetService(NS_PREFSERVICE_CONTRACTID));
>+    
>+    prefBranch->GetComplexValue("intl.accept_languages",
>+                                NS_GET_IID(nsIPrefLocalizedString),
>+                                getter_AddRefs(prefString));
>+    if (prefString) {
>+      prefString->ToString(getter_Copies(langPrefs));
>+    }

It's considered better C++ style to declare variables as late as possible.

>+    if (!langPrefs.IsEmpty()) {
>+      value.StripWhitespace();

Ah, I see.  You call StripWhitespace here.  It seems weird to require the
caller to StripWhitespace for one of the parameters and make the function do it
for the other.	Better to pick one.

>+#ifdef  DEBUG_scooter
>+      printf("Calling SVG_TestLanguage('%s','%s')\n", NS_ConvertUCS2toUTF8(value).get(), 
>+                                                      NS_ConvertUCS2toUTF8(langPrefs).get());
>+#endif
>+      aHasSystemLanguage = SVG_TestLanguage(value, langPrefs);
>+    } else {
>+      // For now, evaluate to true.
>+      NS_WARNING("no default language specified for systemLanguage conditional test");
>+      aHasSystemLanguage = PR_TRUE;
>+    }
>+  } else if ((NS_CONTENT_ATTR_NOT_THERE == rv) || (NS_CONTENT_ATTR_NO_VALUE == rv)) {
>+    aHasSystemLanguage = PR_TRUE;
>+    rv = NS_OK;
>+  }

same comment on rv and else as before

>+
>+  return rv;

which means this should probably be |return NS_OK|.

>+}
>+
>+nsresult
>+nsCSSFrameConstructor::SVGSwitchProcessChildren(nsIPresShell*            aPresShell,

More later, from here.
Comment on attachment 145017 [details] [diff] [review]
Updated patch 3

More comments on things I already went through:

>+static PRBool
>+DashMatchCompare(const nsAString& aAttributeValue,
>+                 const nsAString& aSelectorValue,
>+                 const PRBool aCaseSensitive)

These |const nsAString&| could be changed to |const nsSubstring&| when you move
this.

>+// Test to see if this language is supported
>+static PRBool
>+SVG_TestLanguage(const nsAString& lstr, const nsAString& prefs) {

So that these could be |const nsSubstring&| as well.

And it's probably better to require the StripWhitespace from the caller in both
cases, since that reduces string copying and means you can still declare the
strings |const|.  That said, you could just make them non-const and document
what the function does.
Comment on attachment 145017 [details] [diff] [review]
Updated patch 3

>+nsCSSFrameConstructor::SVGSwitchProcessChildren

>+  ChildIterator iter, last;
>+  for (ChildIterator::Init(aContent, &iter, &last);
>+       (iter != last) && (! isFinished);
>+       ++iter) {

I'm not sure the use of ChildIterator here is correct.	Should the switch act
on its real children or on its XBL anonymous children in the presence of a
binding?  It seems to me that it should act as though all its children other
than the one selected are 'display:none' -- i.e., it should act on its content
tree children rather than its anonymous children.  However, this seems a good
bit harder to implement -- you'd have to shoehorn only the correct child into
the ChildIterator, and then if the XBL binding had an insertion point, you'd
have to recover that correct child and create only it.
Depends on: 242298
How unrendered SVG content behaves does not depend on how unrendered 
content in HTML behaves, and may indeed behave differently.  There is, in fact,
specific language in the SVG spec and draft 1.2 spec regarding things like 
what to do with loading resources and images etc.

In more detail:  Unrendered branches in switch do exist in the document.  They
may be referenced by other parts of the document (such as with a "use") and
they may be maniuplated by DOM.

From http://www.w3.org/TR/SVG12/#conditional-processing-overview

  "Similar to the 'display' property, conditional processing attributes 
   only affect the direct rendering of elements and do not prevent 
   elements from being successfully referenced by other elements 
   (such as via a 'use')."

In addition, if an image resource is not rendered (out of viewport or in
a part of a switch statement that is not rendered), the UA is not obliged
to load or fetch that resource, hence the whole multi-resolution test in 1.2
for doing maps that load the appropriate level of detail and resolution for
the currently viewed area.

It is worth noting that there can be a great deal of SVG content that is
not directly rendered lurking in defs sections that is referenced by patterns,
fills, the use element, and so on.


 
No longer depends on: 242298
> and they may be maniuplated by DOM.

> In addition, if an image resource is not rendered (out of viewport or in
> a part of a switch statement that is not rendered), the UA is not obliged
> to load or fetch that resource

Those statements are in direct contradiction of each other if the image resource
involved is an <html:img>, since the DOM exposes some information about the
image itself.  I suggest bringing this up in the WG -- they are making it very
difficult to usefully mix SVG and XHTML....

In any case, sounds like scripts in all branches should execute, frames in all
branches should load, etc, right?
There is no contradiction.

When the SVG 1.2 draft spec talks about loading images, saying things like:

"SVG 1.2, a user agent is not required to load image data for an image that is
not displayed (e.g. is is outside the initial document viewport)."

( http://www.w3.org/TR/SVG12/#loadingimages )

It's talking about the <svg:image> element.  This means that although the
DOM element for the <svg:image> is present, some of the methods on it may fail
because the image is not loaded.

A similar approach could be taken with <html:img>, but I don't think that's
required.  SVG may behave differently from XHTML.  How XHTML entities 
that are not displayed behave (like frames) doesn't depend upon SVG.  
It's up to XHTML, I think.

For SVG inside a switch branch, it's not displayed and treated like being in 
a <defs> section.   That's not new or radical, <defs> have been around since
SVG 1.0
Attached patch Updated patch #4 (obsolete) — Splinter Review
OK, here is another version of this patch.  I have moved DashMatchCompare into
nsStyleUtil.cpp and removed it from nsCSSStyleSheet.cpp.  I had changed the
signature to use nsSubstring, but some of the calls in nsCSSStyleSheet.cpp
aren't Substrings, so I left them as nsAString.  I also addressed the other
issues that came out in superreview, I think.
Attachment #145017 - Attachment is obsolete: true
Attachment #149554 - Flags: review?
Attachment #149554 - Flags: review? → review?(alecf)
Attachment #149554 - Flags: review?(alecf) → review?(alex)
Comment on attachment 149554 [details] [diff] [review]
Updated patch #4

>+// Test to see if this language is supported
>+static PRBool
>+SVG_TestLanguage(const nsSubstring& lstr, const nsSubstring& prefs) 
>+{
>+  // Compare list to attribute value, which may be a list
>+  // According to the SVG 1.1 Spec (at least as I read it), we should take
>+  // the first attribute value and check it for any matches in the users
>+  // preferences, including any prefix matches.

I think dbaron wanted an in-code comment here about the algorithm being O(M*N). 

>+nsCSSFrameConstructor::TestSVGConditions(nsIContent* aContent,
> [...]
>+  rv = aContent->GetAttr(kNameSpaceID_None, nsSVGAtoms::requiredExtensions, value);
>+
>+  aHasRequiredExtensions = PR_TRUE;
>+  if (NS_CONTENT_ATTR_HAS_VALUE == rv) {
>+    // For now, claim that mozilla's SVG implementation supports
>+    // no extensions.  So, if extensions are required, we don't have
>+    // them available.
>+    aHasRequiredExtensions = PR_FALSE;
>+  }
>+  if (NS_FAILED(rv))
>+    return rv;

You can move the NS_FAILED() check to right after aContent->GetAttr.

>+
>+
>+  // Required Features
>+  aHasRequiredFeatures = PR_TRUE;
>+  rv = aContent->GetAttr(kNameSpaceID_None, nsSVGAtoms::requiredFeatures, value);
>+  if (NS_CONTENT_ATTR_HAS_VALUE == rv) {
>+    aHasRequiredFeatures = NS_SVG_TestFeatures(value);
>+  }
>+  if (NS_FAILED(rv))
>+    return rv;

Same here.
Attachment #149554 - Flags: superreview?(dbaron)
Attachment #149554 - Flags: review?(alex)
Attachment #149554 - Flags: review+
Attachment #145017 - Flags: superreview?(dbaron)
Attached patch Updated patch #5 (obsolete) — Splinter Review
This patch has been brought up to date and all of Alex's changes incorporated. 
Ready for final r/sr (I hope).
Attachment #134555 - Attachment is obsolete: true
Attachment #149554 - Attachment is obsolete: true
Attachment #152740 - Flags: superreview?(dbaron)
You didn't merge in my recent changes to DashMatchCompare, for a start.

But what about the XBL issue?  I'm hesitant to take a patch whose basic design
is incompatible with fully correct implementation.  Or should I just not worry
about that?
(In reply to comment #29)
> But what about the XBL issue?  I'm hesitant to take a patch whose basic design
> is incompatible with fully correct implementation.  Or should I just not worry
> about that?

I'm probably in favour of iterating over the anonymous children as scooter is
doing. 
In particular, we want to be able to do things like:

<myCanvas>
  <mySwitch><myChild1/><myChild2/></mySwitch>
</myCanvas>

where:

<binding id="myCanvas"><content><svg:svg><children/></svg:svg></content></binding>

<binding id="mySwitch">
  <content><svg:switch><children/></svg:content>
</binding>

<binding id="myChild1">
  <content><svg:g requiredFeatures="...">...</svg:g></content>
</binding>

<binding id="myChild2">
  <content><svg:g requiredFeatures="...">...</svg:g></content>
</binding>
(In reply to comment #29)
> You didn't merge in my recent changes to DashMatchCompare, for a start.
> 
Sorry -- I noticed that DashMatchCompare got a lot more usage, but didn't notice
the changes in the routine itself.  I'll get you an updated patch today.

Attached patch Updated patch #6 (obsolete) — Splinter Review
OK, I've modified DashMatchCompare to take a comparator function now.
Attachment #152740 - Attachment is obsolete: true
Attachment #152810 - Flags: superreview?(dbaron)
Attachment #152810 - Attachment is obsolete: true
Attachment #152810 - Flags: superreview?(dbaron)
Attachment #154801 - Flags: superreview?(dbaron)
Attachment #154801 - Flags: superreview?(dbaron) → superreview?(bzbarsky)
Attachment #154801 - Flags: superreview?(bzbarsky) → superreview?(dbaron)
Comment on attachment 154801 [details] [diff] [review]
Refresh of patch against current tree -- no other changes

OK, sr=dbaron on the condition that you send a message to www-svg describing
the question we disagree on (whether, when XBL is bound to an svg:switch, the
switch should operate over the real children or the anonymous children) and
asking what the WG thinks of the issue *before* you land the patch, and that if
the WG disagrees or is silent, you file a new bug depending on bug 242298.
Attachment #154801 - Flags: superreview?(dbaron) → superreview+
This should be the final version, depending on the results of an e-mail
exchange with W3C about the correct handling of switch on XBL anonymous
children.
Attachment #154801 - Attachment is obsolete: true
Checked in.

Checking in content/base/src/nsGenericElement.cpp;
/cvsroot/mozilla/content/base/src/nsGenericElement.cpp,v  <--  nsGenericElement.cpp
new revision: 3.352; previous revision: 3.351
done
Checking in content/html/style/public/nsStyleUtil.h;
/cvsroot/mozilla/content/html/style/public/nsStyleUtil.h,v  <--  nsStyleUtil.h
new revision: 1.11; previous revision: 1.10
done
Checking in content/html/style/src/nsCSSStyleSheet.cpp;
/cvsroot/mozilla/content/html/style/src/nsCSSStyleSheet.cpp,v  <-- 
nsCSSStyleSheet.cpp
new revision: 3.314; previous revision: 3.313
done
Checking in content/html/style/src/nsStyleUtil.cpp;
/cvsroot/mozilla/content/html/style/src/nsStyleUtil.cpp,v  <--  nsStyleUtil.cpp
new revision: 3.55; previous revision: 3.54
done
Checking in content/shared/public/nsSVGAtomList.h;
/cvsroot/mozilla/content/shared/public/nsSVGAtomList.h,v  <--  nsSVGAtomList.h
new revision: 1.11; previous revision: 1.10
done
Checking in layout/html/style/src/Makefile.in;
/cvsroot/mozilla/layout/html/style/src/Makefile.in,v  <--  Makefile.in
new revision: 1.62; previous revision: 1.61
done
Checking in layout/html/style/src/nsCSSFrameConstructor.cpp;
/cvsroot/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp,v  <-- 
nsCSSFrameConstructor.cpp
new revision: 1.970; previous revision: 1.969
done
Checking in layout/html/style/src/nsCSSFrameConstructor.h;
/cvsroot/mozilla/layout/html/style/src/nsCSSFrameConstructor.h,v  <-- 
nsCSSFrameConstructor.h
new revision: 1.156; previous revision: 1.155
done
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Attachment #156842 - Attachment is obsolete: true
Missing files checked in.

cvs commit: Examining .
Checking in Makefile.in;
/cvsroot/mozilla/content/svg/content/src/Makefile.in,v  <--  Makefile.in
new revision: 1.21; previous revision: 1.20
done
RCS file: /cvsroot/mozilla/content/svg/content/src/nsSVGFeatures.cpp,v
done
Checking in nsSVGFeatures.cpp;
/cvsroot/mozilla/content/svg/content/src/nsSVGFeatures.cpp,v  <--  nsSVGFeatures.cpp
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/content/svg/content/src/nsSVGFeaturesList.h,v
done
Checking in nsSVGFeaturesList.h;
/cvsroot/mozilla/content/svg/content/src/nsSVGFeaturesList.h,v  <-- 
nsSVGFeaturesList.h
initial revision: 1.1
done
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: