The default bug view has changed. See this FAQ.

Schema with forward type definitions causing load errors

RESOLVED FIXED

Status

()

Core
Web Services
RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: Steve Speicher, Assigned: Doron Rosenberg (IBM))

Tracking

({fixed1.8.1.1})

Trunk
fixed1.8.1.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Reporter)

Description

11 years ago
User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1) Opera 7.55  [en] (IBM EVV/3.8/EAK01AGF/LE)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060508 Minefield/3.0a1

Schema definition like this (complexType forward referencing another complexType) is the cause of the problem:
  <complexType name="fooType">
    <simpleContent>
      <restriction base="tns:textType">
        <maxLength value="2" />
      </restriction>
    </simpleContent>
  </complexType>

  <complexType name="textType">
    <simpleContent>
      <extension base="string">
      </extension>
    </simpleContent>
  </complexType>


Reproducible: Always
(Reporter)

Comment 1

11 years ago
Created attachment 221350 [details]
test case
(Reporter)

Comment 2

11 years ago
Created attachment 221351 [details]
driver XForm with link to schema with problem
(Reporter)

Comment 3

11 years ago
Created attachment 221352 [details]
schema that works
(Assignee)

Comment 4

11 years ago
Created attachment 221501 [details] [diff] [review]
le patch

this should fix it.  Steve, can you test with your larger forms?

Parts of the code are not schema placeholder (if a type is defined after a user is found) compliant, this makes them behave again.
(Reporter)

Comment 5

11 years ago
(In reply to comment #4)

> Steve, can you test with your larger forms?
> 

Yes and along with patch for bug 337179, works nicely.
(Reporter)

Comment 6

11 years ago
(In reply to comment #5)
> (In reply to comment #4)
> 
> > Steve, can you test with your larger forms?
> > 
> 
> Yes and along with patch for bug 337179, works nicely.
> 

I meant with bug 335366 (though not strictly dependent on it)
(Assignee)

Updated

11 years ago
Assignee: aaronr → doronr
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Updated

11 years ago
Attachment #221501 - Flags: review?(aaronr)

Comment 7

11 years ago
Comment on attachment 221501 [details] [diff] [review]
le patch


>Index: extensions/webservices/schema/src/nsSchemaLoader.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/webservices/schema/src/nsSchemaLoader.cpp,v
>retrieving revision 1.43.2.3
>diff -u -p -1 -0 -r1.43.2.3 nsSchemaLoader.cpp
>--- extensions/webservices/schema/src/nsSchemaLoader.cpp	15 Mar 2006 17:35:52 -0000	1.43.2.3
>+++ extensions/webservices/schema/src/nsSchemaLoader.cpp	9 May 2006 21:45:24 -0000
>@@ -1840,43 +1840,51 @@ nsSchemaLoader::ProcessSimpleContentRest
>  
>   restrictionInst = new nsSchemaRestrictionType(aSchema, EmptyString());
>   if (!restrictionInst) {
>     return NS_ERROR_OUT_OF_MEMORY;
>   }
>   simpleBase = restrictionInst;
>   
>   // The base type must actually be a complex type (which itself must
>   // have a simple base type.
>   nsCOMPtr<nsISchemaComplexType> complexBase = do_QueryInterface(aBaseType);
>-  if (!complexBase) {
>-    nsAutoString baseStr;
>-    rv = aBaseType->GetName(baseStr);
>-    NS_ENSURE_SUCCESS(rv, rv);
>-
>-    nsAutoString errorMsg;
>-    errorMsg.AppendLiteral("Failure processing schema, base type \"");
>-    errorMsg.Append(baseStr);
>-    errorMsg.AppendLiteral("\" of restriction must be a complex type ");
>-    errorMsg.AppendLiteral("which itself must be based on a simple type");
>+    if (!complexBase) {
>+    // if base type is a place holder, this is ok
>+    PRUint16 schemaType;
>+    aBaseType->GetSchemaType(&schemaType);

nit: indentation

>+
>+    if (schemaType == nsISchemaType::SCHEMA_TYPE_PLACEHOLDER) {
>+      simpleBase = do_QueryInterface(aBaseType);

you don't check the return type from GetSchemaType and you don't assign an initial value to schemaType.  You like walking the razor's edge!  fix please.

>+    } else {
>+      nsAutoString baseStr;
>+      rv = aBaseType->GetName(baseStr);
>+      NS_ENSURE_SUCCESS(rv, rv);
> 
>-    NS_SCHEMALOADER_FIRE_ERROR(NS_ERROR_SCHEMA_INVALID_TYPE_USAGE, errorMsg);
>+      nsAutoString errorMsg;
>+      errorMsg.AppendLiteral("Failure processing schema, base type \"");
>+      errorMsg.Append(baseStr);
>+      errorMsg.AppendLiteral("\" of restriction must be a complex type ");
>+      errorMsg.AppendLiteral("which itself must be based on a simple type");
> 
>-    return NS_ERROR_SCHEMA_INVALID_TYPE_USAGE;
>-  }
>+      NS_SCHEMALOADER_FIRE_ERROR(NS_ERROR_SCHEMA_INVALID_TYPE_USAGE, errorMsg);
> 
>-  nsCOMPtr<nsISchemaSimpleType> parentSimpleBase;
>-  complexBase->GetSimpleBaseType(getter_AddRefs(parentSimpleBase));
>+      return NS_ERROR_SCHEMA_INVALID_TYPE_USAGE;
>+    }
>+  } else {
>+    nsCOMPtr<nsISchemaSimpleType> parentSimpleBase;
>+    complexBase->GetSimpleBaseType(getter_AddRefs(parentSimpleBase));
>   
>-  if (parentSimpleBase) {
>-    rv = restrictionInst->SetBaseType(parentSimpleBase);
>-    if (NS_FAILED(rv)) {
>-      return rv;
>+    if (parentSimpleBase) {
>+      rv = restrictionInst->SetBaseType(parentSimpleBase);
>+      if (NS_FAILED(rv)) {
>+        return rv;
>+      }

nit: NS_ENSURE_SUCCESS(rv, rv)

nothing major that I noticed.  r=me
Attachment #221501 - Flags: review?(aaronr) → review+
(Assignee)

Updated

11 years ago
Attachment #221501 - Flags: review?(allan)

Comment 8

11 years ago
Comment on attachment 221501 [details] [diff] [review]
le patch

I do not see something obviously wrong, except for the remarks that Aaron had. But I do not really feel qualified for r+ in that part of the code....
Attachment #221501 - Flags: review?(allan)
(Assignee)

Updated

11 years ago
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch

Updated

11 years ago
Component: XForms → Web Services
OS: Windows XP → All
Hardware: PC → All

Comment 9

11 years ago
Comment on attachment 221501 [details] [diff] [review]
le patch

great to have for the next 1.8.0.x release
Attachment #221501 - Flags: approval1.8.0.8?

Updated

11 years ago
Attachment #221501 - Flags: approval1.8.1?
Summary: Schema with forward type defintions causing load errors → Schema with forward type definitions causing load errors

Comment 10

11 years ago
Comment on attachment 221501 [details] [diff] [review]
le patch

Doesn't meet criteria for RC2.   i.e. "Show Stopper Bugs" - major regression, crash, security issue.  We can pickup in a follow-on release.
Attachment #221501 - Flags: approval1.8.1? → approval1.8.1-
Comment on attachment 221501 [details] [diff] [review]
le patch

1.8.0.8 is to keep 1.8.0.x security fixes in-line with the FF2 release, we're not taking things they aren't.
Attachment #221501 - Flags: approval1.8.0.8? → approval1.8.0.8-

Comment 12

11 years ago
Comment on attachment 221501 [details] [diff] [review]
le patch

Now that FF 2 is out, can we get this in?
Attachment #221501 - Flags: approval1.8.1.1?
Comment on attachment 221501 [details] [diff] [review]
le patch

Doesn't appear to have a super review, This is going to have to wait.
Attachment #221501 - Flags: approval1.8.1.2?
Attachment #221501 - Flags: approval1.8.1.1?
Attachment #221501 - Flags: approval1.8.1.1-
(Assignee)

Comment 14

11 years ago
Patch was written by module owner and was simple enough to not need a sr.
(In reply to comment #14)
> Patch was written by module owner and was simple enough to not need a sr.

Rerequest approval1.8.1.1, or your message will never be seen by the drivers.

Comment 16

11 years ago
Comment on attachment 221501 [details] [diff] [review]
le patch

Given Doron's last comment, approved for 1.8 branch, a= jay for drivers.
Attachment #221501 - Flags: approval1.8.1.2?
Attachment #221501 - Flags: approval1.8.1.1-
Attachment #221501 - Flags: approval1.8.1.1+
(Assignee)

Updated

11 years ago
Keywords: fixed1.8.1.1

Comment 17

10 years ago
doesn't sound eligible for 1.8.0 branch, so removing xf-to-branch
Whiteboard: xf-to-branch
You need to log in before you can comment on or make changes to this bug.