578142 - Make XForms work after XPCOM registration changes

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Philipp Wagner [:imphil]]

The XPCOM registration changes landed on mozilla-central (bug 568691) and will soon appear on mozilla-2.0. Get the XForms extension ready for it.

Comment 1

[Philipp Wagner [:imphil]]

Attachment: [xforms] WIP patch

Comment 2

[Philipp Wagner [:imphil]]

Attachment: [schema-validation] patch v1

Comment 3

[Philipp Wagner [:imphil]]

so, here are the first tries... 

Prerequisites: 
* we're talking about mozilla-central
* apply all patches from bug 547652

What works
* the XTF-stuff looks ok (xforms and schema-validation is loaded), the calculator example (http://www.mozilla.org/projects/xforms/samples/calc_svg_1010.xhtml) actually works as expected

Known problems:
* There are tons of messages on the error console like "While registering XPCOM module, trying to re-register CID '/home/philipp/src/mozilla-central/obj-debug/dist/bin/extensions/{cf2812dc-6a7c-4402-b639-4d277dac4c36}/components/libschemavalidation.so' already registered by {45c28fed-b8d3-4fba-8105-e9659f138c0f}." Those appear with different "already registered by" CIDs, it looks like all of the ones used in nsSchemaValidatorModule.cpp.

* Our XPath functions (like instance() ) are not found. I couldn't figure out how the XPath engine finds our functions in the first place, probably the nsIClassInfo changes broke something here. Who knows how it worked and how we get it working again? I've attached a testcase that shows that problem.

Comment 4

[Philipp Wagner [:imphil]]

Attachment: testcase if our xpath extension functions are found

Comment 5

[Philipp Wagner [:imphil]]

OK, follow-up to known problem 1: the error message is wrong, it should read

While registering XPCOM module /home/philipp/src/mozilla-central/obj-debug/dist/bin/extensions/{cf2812dc-6a7c-4402-b639-4d277dac4c36}/components/libschemavalidation.so, trying to re-register CID '{eced2af3-fde9-4575-b5a4-e1c830b24611}' already registered by /home/philipp/src/mozilla-central/obj-debug/dist/bin/components/libschemavalidation.so.

So that's a probably harmless problem only happening during development when libschemavalidation.so is around twice.

bug 578237 should fix the error message



re problem 2: it's somewhere in the component registry, xpath seems to be innocent. still looking.

Comment 6

[Philipp Wagner [:imphil]]

OK, debugged problem 2 down a bit, but now I'm getting more and more lost ...

If the XPath implementation tries to resolve a function call, it goes like that (some steps left out)


TX_ResolveFunctionCallXPCOM
  /content/xslt/src/xpath/txXPCOMExtensionFunction.cpp:271

LookupFunction
  /content/xslt/src/xpath/txXPCOMExtensionFunction.cpp:197

nsComponentManagerImpl::GetClassObjectByContractID(contractID="@mozilla.org/xforms-xpath-functions;1", aIID=NS_ICLASSINFO_IID)
  /xpcom/components/nsComponentManager.cpp:1152


1152     nsCOMPtr<nsIFactory> factory = FindFactory(contractID, strlen(contractID));
1153     if (!factory)
1154         return NS_ERROR_FACTORY_NOT_REGISTERED;
1155 
1156     rv = factory->QueryInterface(aIID, aResult);


Now inside GetClassObjectByContractID, the code gets the nsIFactory for our nsXFormsXPathFunctions (a nsIGenericFactory as we don't specify our own one) and tries to QueryInterface it to nsIClassInfo - which fails (seems logical). 

Questions are:
1) Why does a method named GetClassObjectByContractID return the factory instead of an object of nsXFormsXPathFunctions?

2) How has this ever worked in the previous code (the nsIClassInfo-stuff was somehow in the registration, see http://hg.mozilla.org/xforms/file/f51a343e8228/nsXFormsModule.cpp#l147. From what I've seen in other commits it seemed enough to replace it with the new macros as I've done in the patch)

3) The changes in http://hg.mozilla.org/mozilla-central/file/90afd1e80d77/content/xslt/src/xslt/txEXSLTRegExFunctions.js suggest to create a own factory which supports nsIClassInfo in QueryInterface - is this the way to go or only a necessity in JS?


I hope someone with more XPCOM knowledge can bring some light into this...

Comment 7

[Benjamin Smedberg]

Factory objects used to also be the classinfo for a class. This is no longer the case. I don't really understand what xforms is doing here, but you'll have to modify it to get the classinfo or whatever it's doing using another method.

Comment 8

[Philipp Wagner [:imphil]]

Attachment: [xforms] patch v1

Comment 9

[Philipp Wagner [:imphil]]

Thanks Benjamin! I now create a own factory class that supports nsIClassInfo. It's essentially the same approach as taken in txEXSLTRegExFunctions.js.

I didn't find any more regressions, let's hope it stays that way. With this patch XForms compiles on mozilla-central.

Comment 10

[aaronr]

We are probably the only users of the xpath extension function facility currently, but you should probably check with peterv to see if they had any desire to change that area of code now that factories aren't the classinfo anymore.  Otherwise a comment should probably be added in the xpath code as to this new requirement for xpath function factories to implement that interface.

Comment 11

[Philipp Wagner [:imphil]]

Well, txEXSLTRegExFunctions.js seems to use the same interface. 

peterv, do you want to/can you change the code in LookupFunction() or should we continue to implement or own nsIFactory (like in the patch or in txEXSLTRegExFunctions.js)?

Comment 12

[Peter Van der Beken [:peterv]]

I don't really care either way. It's trivial to change LookupFunction so it does |nsCOMPtr<nsISupports> helper = do_GetService(aContractID);| and do_QueryInterface to get the classinfo instead of using do_GetClassObject. Just make LookupFunction return the helper in an out param (after calling QueryInterface(iid)) and remove the call to CallGetService in TX_ResolveFunctionCallXPCOM.

Comment 13

[Philipp Wagner [:imphil]]

looks like the nicer solution as it saves a lot of code to create the factory. Do you know of other users other than the txEXSLTRegExFunctions.js?

Comment 14

[Peter Van der Beken [:peterv]]

No.

Comment 15

[Peter Van der Beken [:peterv]]

But note that existing users of this API (txEXSLTRegExFunctions.js) wouldn't need any changes.

Comment 16

[Philipp Wagner [:imphil]]

Attachment: xpath changes

Comment 17

[Philipp Wagner [:imphil]]

Attachment: [xforms] patch v2 (requires the xpath changes)

Comment 18

[Philipp Wagner [:imphil]]

Thanks Peter, it looks much nicer now (saves around 120 lines)

Comment 19

[alexander :surkov (:asurkov)]

Comment on attachment 456912 [details] [diff] [review]
[schema-validation] patch v1

at quick glance looks ok, however it's worth to find reviewer more familiar with xpcom components registration code.

Comment 20

[Peter Van der Beken [:peterv]]

Comment on attachment 457539 [details] [diff] [review]
xpath changes

>+    nsresult rv = LookupFunction(aContractID.get(), aName, iid, methodIndex,
>+                                getter_AddRefs(helper));

Nit: line up the |getter_...| with |(aCon...|.

Comment 21

[Philipp Wagner [:imphil]]

Attachment: [for checkin] xpath changes v1.1 (only whitespace fixes, r=peterv)

approval2.0 requested because we need this patch to get XForms working after the XPCOM changes. I have no access to a tryserver, but it should be fairly safe.

Comment 22

[Benjamin Smedberg]

Comment on attachment 459001 [details] [diff] [review]
[for checkin] xpath changes v1.1 (only whitespace fixes, r=peterv)

Is this covered by existing tests? If not, this will need test coverage before landing. Please re-request approval as appropriate.

Comment 23

[Philipp Wagner [:imphil]]

What needs to be tested? That the API works as expected or that the in-tree consumers of this API continue to work?

Comment 24

[Benjamin Smedberg]

The TX_ResolveFunctionCallXPCOM codepath needs to be tested somehow. It's not clear to me whether there are in-tree consumers of txXPCOMExtensionFunction.cpp or whether XForms is the consumer. If it's just XForms, you'll need to write an API test.

Comment 25

[Peter Van der Beken [:peterv]]

EXSLT is the other consumer, we have at least two mochitests for EXSLT so those should cover it.

Comment 26

[Peter Van der Beken [:peterv]]

Actually, that's wrong. We have mochitests for exslt:common, but that doesn't use txXPCOMExtensionFunction.cpp. We need a mochitest for exslt:regexp (see content/xslt/tests/mochitest/test_bug551654.html or content/xslt/tests/mochitest/test_bug551412.html for examples of exslt mochitests).

Comment 27

[Philipp Wagner [:imphil]]

When writing the test I saw that the changes broke the regexp functions, so I updated those as well. To keep this bug focused on xforms I put the new patch into bug 581305.

Comment 28

[alexander :surkov (:asurkov)]

Comment on attachment 457541 [details] [diff] [review]
[xforms] patch v2 (requires the xpath changes)

Philipp, could you describe the patch changes to make my review easier please?

Comment 29

[Philipp Wagner [:imphil]]

Comment on attachment 457541 [details] [diff] [review]
[xforms] patch v2 (requires the xpath changes)

I hope the following comments help ...

>From: Philipp Wagner <[e-mail]>
>
>xpcom changes v2
>
>diff --git a/Makefile.in b/Makefile.in
>--- a/Makefile.in
>+++ b/Makefile.in
>+DEFINES += -DXFORMS_SHARED_LIBRARY=$(SHARED_LIBRARY) \
>+           -DSCHEMAVALIDATION_SHARED_LIBRARY=$(DLL_PREFIX)schemavalidation$(DLL_SUFFIX) \
>+           -DTARGET_PLATFORM=$(OS_TARGET)_$(TARGET_XPCOM_ABI)

these are used in chrome.manifest (generated from jar.mn below) for component registration. Suggestion from bsmedberg (http://groups.google.com/group/mozilla.dev.platform/msg/01f11ad27947d627)


>diff --git a/jar.mn b/jar.mn
>--- a/jar.mn
>+++ b/jar.mn
>@@ -1,13 +1,21 @@
>+#filter substitution
> xforms.jar:
>-% overlay	chrome://browser/content/preferences/preferences.xul	chrome://xforms/content/xforms-prefs.xul
>-% content	xforms	%content/xforms/ contentaccessible=yes
>-% locale	xforms	en-US	%locale/en-US/xforms/
>-% skin	xforms	classic/1.0	%skin/xforms/
>+% overlay chrome://browser/content/preferences/preferences.xul chrome://xforms/content/xforms-prefs.xul
>+% content xforms %content/xforms/ contentaccessible=yes
>+% locale xforms en-US %locale/en-US/xforms/
>+% skin xforms classic/1.0 %skin/xforms/
>+% interfaces components/schemavalidation.xpt
>+% interfaces components/xforms.xpt
>+% binary-component components/@XFORMS_SHARED_LIBRARY@ ABI=@TARGET_PLATFORM@
>+% binary-component components/@SCHEMAVALIDATION_SHARED_LIBRARY@ ABI=@TARGET_PLATFORM@
>+% component {26d69f7e-f7cf-423d-afb9-43d8a9ebf3ba} components/nsSchemaValidatorRegexp.js
>+% contract @mozilla.org/xmlextras/schemas/schemavalidatorregexp;1 {26d69f7e-f7cf-423d-afb9-43d8a9ebf3ba}
>+% category profile-after-change nsSchemaValidatorRegexp	@mozilla.org/xmlextras/schemas/schemavalidatorregexp;1

register our components and interfaces as they are not auto-registered any more. See https://developer.mozilla.org/en/XPCOM/XPCOM_changes_in_Gecko_2.0 for details. I added the ABI to make it easier for my scripts to created merged XPIs that contain binary parts for different platforms. There is no functional difference (obviously the binaries wouldn't work on other platforms anyway).


>diff --git a/nsXFormsDOMEvent.cpp b/nsXFormsDOMEvent.cpp
>--- a/nsXFormsDOMEvent.cpp
>+++ b/nsXFormsDOMEvent.cpp
>@@ -180,8 +180,24 @@ nsXFormsDOMEvent::GetInternalNSEvent()
> }
> 
> NS_METHOD
> nsXFormsDOMEvent::SetTrusted(PRBool aTrusted)
> {
>   nsCOMPtr<nsIPrivateDOMEvent> privEvent = do_QueryInterface(mInner);
>   return privEvent->SetTrusted(aTrusted);
> }
>+
>+void
>+nsXFormsDOMEvent::Serialize(IPC::Message* aMsg,
>+                        PRBool aSerializeInterfaceType)
>+{
>+  nsCOMPtr<nsIPrivateDOMEvent> privEvent = do_QueryInterface(mInner);
>+  privEvent->Serialize(aMsg, aSerializeInterfaceType);
>+}
>+
>+PRBool
>+nsXFormsDOMEvent::Deserialize(const IPC::Message* aMsg, void** aIter)
>+{
>+  nsCOMPtr<nsIPrivateDOMEvent> privEvent = do_QueryInterface(mInner);
>+  return privEvent->Deserialize(aMsg, aIter);
>+}

nsIPrivateDOMEvent now requires those two methods. Simply forward them the same way we did the other methods. Not really XPCOM related, just didn't want to open another bug for those little changes. If that's a problem, I can open another bug report as well.

>diff --git a/nsXFormsModule.cpp b/nsXFormsModule.cpp
>--- a/nsXFormsModule.cpp
>+++ b/nsXFormsModule.cpp
>-#include "nsIGenericFactory.h"
>+#include "mozilla/ModuleUtils.h"
> #include "nsXFormsElementFactory.h"
> #include "nsXFormsUtilityService.h"
> #include "nsXFormsAtoms.h"
> #include "nsXFormsModelElement.h"
> #include "nsXFormsUtils.h"
>-#include "nsICategoryManager.h"
>-#include "nsIServiceManager.h"
>+#include "nsXFormsXPathFunctions.h"
> #include "nsIClassInfoImpl.h"
>-#include "nsXFormsXPathFunctions.h"
>-#include "nsServiceManagerUtils.h"
>-
>-// bb0d9c8b-3096-4b66-92a0-6c1ddf80e65f
>-#define NS_XFORMSUTILITYSERVICE_CID \
>-{ 0xbb0d9c8b, 0x3096, 0x4b66, { 0x92, 0xa0, 0x6c, 0x1d, 0xdf, 0x80, 0xe6, 0x5f }}
>-
>-#define NS_XFORMSUTILITYSERVICE_CONTRACTID \
>-"@mozilla.org/xforms-utility-service;1"
>-
>-#define XFORMSXPATHFUNCTIONS_CID \
>-{ 0x8edc8cf1, 0x69a3, 0x11d9, { 0x97, 0x91, 0x00, 0x0a, 0x95, 0xdc, 0x23, 0x4c } }
>-#define XFORMSXPATHFUNCTIONS_CONTRACTID \
>-"@mozilla.org/xforms-xpath-functions;1"
>+#include "mozilla/Module.h"
>+#include "nsIFactory.h"

CID and CONTRACTID are now in their corresponding implementation headers, like it seems to be usually done.

> NS_GENERIC_FACTORY_CONSTRUCTOR(nsXFormsElementFactory)
> NS_GENERIC_FACTORY_CONSTRUCTOR(nsXFormsUtilityService)
> NS_GENERIC_FACTORY_CONSTRUCTOR(nsXFormsXPathFunctions)
>-NS_DECL_CLASSINFO(nsXFormsXPathFunctions)

that one does not exist any more, it's now done nsXFormsXPathFunctions

>-static NS_IMETHODIMP
>-RegisterXFormsModule(nsIComponentManager *aCompMgr,
>-                     nsIFile *aPath,
>-                     const char *aRegistryLocation,
>-                     const char *aComponentType,
>-                     const nsModuleComponentInfo *aInfo)
>-{
>-#ifdef DEBUG
>-  printf("XFORMS Module: Registering\n");
>-#endif
>+NS_DEFINE_NAMED_CID(NS_XFORMSELEMENTFACTORY_CID);
>+NS_DEFINE_NAMED_CID(NS_XFORMSUTILITYSERVICE_CID);
>+NS_DEFINE_NAMED_CID(NS_XFORMSXPATHFUNCTIONS_CID);

gives us the k... constants used below

>-  nsCOMPtr<nsICategoryManager> catman =
>-    do_GetService(NS_CATEGORYMANAGER_CONTRACTID);
> 
>-  if (!catman)
>-    return NS_ERROR_FAILURE;
>-
>-  nsCString previous;
>-  nsresult rv =
>-    catman->AddCategoryEntry(NS_DOMNS_FEATURE_PREFIX "org.w3c.xforms.dom",
>-                             "1.0",
>-                             NS_XTF_ELEMENT_FACTORY_CONTRACTID_PREFIX NS_NAMESPACE_XFORMS,
>-                             PR_TRUE, PR_TRUE, getter_Copies(previous));
>-  NS_ENSURE_SUCCESS(rv, rv);
>-
>-  rv =
>-    catman->AddCategoryEntry(NS_DOMNS_FEATURE_PREFIX NS_XFORMS_INSTANCE_OWNER,
>-                             "1.0",
>-                             NS_XTF_ELEMENT_FACTORY_CONTRACTID_PREFIX NS_NAMESPACE_XFORMS,
>-                             PR_TRUE, PR_TRUE, getter_Copies(previous));
>-  NS_ENSURE_SUCCESS(rv, rv);
>-
>-  return catman->AddCategoryEntry("agent-style-sheets",
>-                                  "xforms stylesheet",
>-                                  "chrome://xforms/content/xforms.css",
>-                                  PR_TRUE, PR_TRUE, getter_Copies(previous));
>-}
>-
>-static NS_IMETHODIMP
>-UnregisterXFormsModule(nsIComponentManager *aCompMgr,
>-                       nsIFile *aPath,
>-                       const char *aRegistryLocation,
>-                       const nsModuleComponentInfo *aInfo)
>-{
>-#ifdef DEBUG
>-  printf("XFORMS Module: Unregistering\n");
>-#endif
>-
>-  nsXFormsUtils::Shutdown();
>-
>-  nsCOMPtr<nsICategoryManager> catman =
>-    do_GetService(NS_CATEGORYMANAGER_CONTRACTID);
>-
>-  if (!catman)
>-    return NS_ERROR_FAILURE;
>-
>-  catman->DeleteCategoryEntry(NS_DOMNS_FEATURE_PREFIX "org.w3c.xforms.dom",
>-                              "1.0", PR_TRUE);
>-
>-  catman->DeleteCategoryEntry(NS_DOMNS_FEATURE_PREFIX NS_XFORMS_INSTANCE_OWNER,
>-                              "1.0", PR_TRUE);
>-
>-  return catman->DeleteCategoryEntry("agent-style-sheets",
>-                                     "xforms stylesheet", PR_TRUE);
>-}

registration with the category manager is now done below in kXFormsCategories. Everything else is in the new Startup/Shutdown methods below.

>-static const nsModuleComponentInfo components[] = {
>-  { "XForms element factory",
>-    NS_XFORMSELEMENTFACTORY_CID,
>-    NS_XTF_ELEMENT_FACTORY_CONTRACTID_PREFIX NS_NAMESPACE_XFORMS,
>-    nsXFormsElementFactoryConstructor,
>-    RegisterXFormsModule,
>-    UnregisterXFormsModule
>-  },
>-  { "XForms Utility Service",
>-    NS_XFORMSUTILITYSERVICE_CID,
>-    NS_XFORMSUTILITYSERVICE_CONTRACTID,
>-    nsXFormsUtilityServiceConstructor
>-  },
>-  { "XForms XPath extension functions",
>-    XFORMSXPATHFUNCTIONS_CID,
>-    XFORMSXPATHFUNCTIONS_CONTRACTID,
>-    nsXFormsXPathFunctionsConstructor,
>-    nsnull, nsnull, nsnull,
>-    NS_CI_INTERFACE_GETTER_NAME(nsXFormsXPathFunctions),
>-    nsnull,
>-    &NS_CLASSINFO_NAME(nsXFormsXPathFunctions)
>-  }

replaced with the new structs below, no real functional changes. The xpath extension functions are special, because the nsIClassInfo (required by the extensible xpath implementation) used to be in the nsIFactory. Now it's in the in nsXFormsXPathFunctions itself. This requires some changes to XPath in bug 581305 and was suggested by peterv.

>+const mozilla::Module::CIDEntry kXFormsCIDs[] = {
>+  { &kNS_XFORMSELEMENTFACTORY_CID, false, NULL, nsXFormsElementFactoryConstructor },
>+  { &kNS_XFORMSUTILITYSERVICE_CID, false, NULL, nsXFormsUtilityServiceConstructor },
>+  { &kNS_XFORMSXPATHFUNCTIONS_CID, false, NULL, nsXFormsXPathFunctionsConstructor },
>+  { NULL }
> };
> 
>-PR_STATIC_CALLBACK(nsresult)
>-XFormsModuleCtor(nsIModule* aSelf)
>+const mozilla::Module::ContractIDEntry kXFormsContracts[] = {
>+  { NS_XTF_ELEMENT_FACTORY_CONTRACTID_PREFIX NS_NAMESPACE_XFORMS, &kNS_XFORMSELEMENTFACTORY_CID },
>+  { NS_XFORMSUTILITYSERVICE_CONTRACTID, &kNS_XFORMSUTILITYSERVICE_CID },
>+  { NS_XFORMSXPATHFUNCTIONS_CONTRACTID, &kNS_XFORMSXPATHFUNCTIONS_CID },
>+  { NULL }
>+};
>+
>+const mozilla::Module::CategoryEntry kXFormsCategories[] = {
>+  { NS_DOMNS_FEATURE_PREFIX "org.w3c.xforms.dom",
>+    "1.0",
>+    NS_XTF_ELEMENT_FACTORY_CONTRACTID_PREFIX NS_NAMESPACE_XFORMS },
>+
>+  { NS_DOMNS_FEATURE_PREFIX NS_XFORMS_INSTANCE_OWNER,
>+    "1.0",
>+    NS_XTF_ELEMENT_FACTORY_CONTRACTID_PREFIX NS_NAMESPACE_XFORMS },
>+
>+  { "agent-style-sheets",
>+    "xforms stylesheet",
>+    "chrome://xforms/content/xforms.css" },
>+
>+  { NULL }
>+};
>+
>+static nsresult
>+nsXFormsStartup()
> {
>   nsXFormsAtoms::InitAtoms();
>   nsXFormsUtils::Init();
>   nsXFormsModelElement::Startup();
>   return NS_OK;
> }
> 
>-NS_IMPL_NSGETMODULE_WITH_CTOR(xforms, components, XFormsModuleCtor)
>+static void
>+nsXFormsShutdown()
>+{
>+  nsXFormsUtils::Shutdown();
>+}

simular to the old Register/Unregister methods, only use more common naming and remove all component manager stuff.

>+
>+static const mozilla::Module kXFormsModule = {
>+  mozilla::Module::kVersion,
>+  kXFormsCIDs,
>+  kXFormsContracts,
>+  kXFormsCategories,
>+  NULL,
>+  nsXFormsStartup,
>+  nsXFormsShutdown
>+};
>+
>+NSMODULE_DEFN(nsXFormsModule) = &kXFormsModule;

>diff --git a/nsXFormsXPathFunctions.cpp b/nsXFormsXPathFunctions.cpp
>--- a/nsXFormsXPathFunctions.cpp
>+++ b/nsXFormsXPathFunctions.cpp
>+NS_IMPL_CLASSINFO(nsXFormsXPathFunctions, NULL, 0, NS_XFORMSXPATHFUNCTIONS_CID)

replaces NS_DECL_CLASSINFO from nsXFormsModule

Comment 30

[alexander :surkov (:asurkov)]

Comment on attachment 457541 [details] [diff] [review]
[xforms] patch v2 (requires the xpath changes)

thanks for the patch and detailed description. looks fine with me, r=me

Comment 31

[Philipp Wagner [:imphil]]

Attachment: [schema-validation] v1.1

remove unnecessary profile-after-change listener

Comment 32

[Philipp Wagner [:imphil]]

Attachment: [xforms] v2.1

Now *.manifest files are auto-generated and included into the chrome.manifest by the build system. If we declare the components twice, Firefox crashes (bug 589992). The new patch versions fix this by not declaring the components in jar.mn any more (as it was before) and let the auto-generation do its work. It's the nicer solution anyway.

Alex: The changes are minor, but I still didn't want to carry over the r+ flags.

Comment 33

[alexander :surkov (:asurkov)]

Comment on attachment 474412 [details] [diff] [review]
[schema-validation] v1.1

rs=me if you're fine with it

Comment 34

[alexander :surkov (:asurkov)]

Comment on attachment 474413 [details] [diff] [review]
[xforms] v2.1

rs=me if you're fine with it

Comment 35

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 474412 [details] [diff] [review]
[schema-validation] v1.1

rs=me

Comment 36

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 474413 [details] [diff] [review]
[xforms] v2.1

>+
>+void
>+nsXFormsDOMEvent::Serialize(IPC::Message* aMsg,
>+                        PRBool aSerializeInterfaceType)
Fix the indentation. PRBool should be under IPC

Comment 37

[Philipp Wagner [:imphil]]

pushed http://hg.mozilla.org/xforms/rev/3bd9353a5347
http://hg.mozilla.org/schema-validation/rev/ea31fd50c36b

(my first two pushes! I hope I didn't miss anything)

I'll leave this open until bug 581305 is fixed, as we currently don't compile without the patch there.

Comment 38

[alexander :surkov (:asurkov)]

(In reply to comment #37)
> pushed http://hg.mozilla.org/xforms/rev/3bd9353a5347
> http://hg.mozilla.org/schema-validation/rev/ea31fd50c36b
> 
> (my first two pushes! I hope I didn't miss anything)

congrats! usually you don't until you have new files (that's common thing that you forget to add them) :)