525730 - Update XForms for trunk version.

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Sergey Reymerov]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; ru; rv:1.9.1.4) Gecko/20091028 Ubuntu/9.10 (karmic) Firefox/3.5.4
Build Identifier: 3.7pre1

Mozilla core changed in past time (some refactoring). So XForms needs to update for new Mozilla interfaces. 

Reproducible: Always

Comment 1

[Sergey Reymerov]

Attachment: solution

have a problem with select1 that in another bug

Comment 2

[alexander :surkov (:asurkov)]

Is this one dupe of bug 502323?

Comment 4

[Philipp Wagner [:imphil]]

Yes, but this one has a at least partially working patch (it doesn't compile yet, I'll look into it later).

Btw Sergey: do you mean trunk (mozilla-central) or mozilla-1.9.2 (will be Firefox 3.6)?

Comment 5

[Sergey Reymerov]

Yes, I mean trunk.

Comment 6

[alexander :surkov (:asurkov)]

(In reply to comment #4)
> Yes, but this one has a at least partially working patch (it doesn't compile
> yet, I'll look into it later).

Do we need working xforms version for 3.6?

Comment 7

[Philipp Wagner [:imphil]]

Of course :-) I was thinking this one is for 1.9.2 as well, that's why I marked the other bug wrongly as dup. I wanted to look into XForms for FF 3.6 this weekend, it should be somewhere between the current status and this patch.

Comment 8

[alexander :surkov (:asurkov)]

(In reply to comment #7)
> Of course :-) I was thinking this one is for 1.9.2 as well, that's why I marked
> the other bug wrongly as dup. I wanted to look into XForms for FF 3.6 this
> weekend, it should be somewhere between the current status and this patch.

Ok, Sergey does it work for you and if so then could you update your patch to gecko 1.9.2 please?

Comment 9

[Philipp Wagner [:imphil]]

Comment on attachment 409588 [details] [diff] [review]
solution

>diff -r 3478e987965d nsXFormsSwitchElement.cpp
>--- a/nsXFormsSwitchElement.cpp	Thu Sep 10 11:15:57 2009 +0800
>+++ b/nsXFormsSwitchElement.cpp	Sun Nov 01 20:31:26 2009 +0800
>@@ -191,7 +193,7 @@
>   mSelected = firstCase;
>   nsCOMPtr<nsIXFormsCaseElement> selected(do_QueryInterface(mSelected));
>   if (selected) {
>-    nsresult rv = selected->SetSelected(PR_TRUE);
>+    selected->SetSelected(PR_TRUE);

why that? that causes a compile-failure here.

Also, schema-validation needs some of the same fixes. Could you look into that as well?

Comment 10

[Sergey Reymerov]

Attachment: Schema validation patch

previously added patch needs this

Comment 11

[alexander :surkov (:asurkov)]

Sergey, you shouldn't set r+ to your patches, you should ask xforms and schema-validation peers for review.

Comment 12

[Sergey Reymerov]

(In reply to comment #9)
> (From update of attachment 409588 [details] [diff] [review])
> >diff -r 3478e987965d nsXFormsSwitchElement.cpp
> >--- a/nsXFormsSwitchElement.cpp	Thu Sep 10 11:15:57 2009 +0800
> >+++ b/nsXFormsSwitchElement.cpp	Sun Nov 01 20:31:26 2009 +0800
> >@@ -191,7 +193,7 @@
> >   mSelected = firstCase;
> >   nsCOMPtr<nsIXFormsCaseElement> selected(do_QueryInterface(mSelected));
> >   if (selected) {
> >-    nsresult rv = selected->SetSelected(PR_TRUE);
> >+    selected->SetSelected(PR_TRUE);
> 
> why that? that causes a compile-failure here.
> 
> Also, schema-validation needs some of the same fixes. Could you look into that
> as well?
why? what error was occurred?
I don't have error in that place. 
Can you remove that piece of patch and try again? I made that only for reduce warnings (unusable varible rv)

Comment 13

[Philipp Wagner [:imphil]]

(In reply to comment #12)
> (In reply to comment #9)
> > (From update of attachment 409588 [details] [diff] [review] [details])
> > >diff -r 3478e987965d nsXFormsSwitchElement.cpp
> > >--- a/nsXFormsSwitchElement.cpp	Thu Sep 10 11:15:57 2009 +0800
> > >+++ b/nsXFormsSwitchElement.cpp	Sun Nov 01 20:31:26 2009 +0800
> > >@@ -191,7 +193,7 @@
> > >   mSelected = firstCase;
> > >   nsCOMPtr<nsIXFormsCaseElement> selected(do_QueryInterface(mSelected));
> > >   if (selected) {
> > >-    nsresult rv = selected->SetSelected(PR_TRUE);
> > >+    selected->SetSelected(PR_TRUE);
> > 
> > why that? that causes a compile-failure here.
> > 
> why? what error was occurred?

rv is used in nsXFormsSwitchElement.cpp:202:

>    NS_WARN_IF_FALSE(mAddingChildren || NS_SUCCEEDED(rv),
>                     "Failed to select case");

Comment 14

[Sergey Reymerov]

Attachment: XForms patch

remove error that was in patch before

Comment 15

[Sergey Reymerov]

I occured new problem in bug 525735 after applying that patch

Comment 16

[Dion Sole]

Just to comment on the closed bug 502323, no I don't have a patch unfortunately. My system was reinstalled some time ago and I didn't remember the mozilla tree...

Comment 17

[Sergey Reymerov]

Bug 525735 is not associated with that bug. Bugs are not related.

Comment 18

[Sergey Reymerov]

I tested that patches in mozilla 3.6 (also I was import patch from bug 525735). This patches are suitable for moz 3.6.

Comment 19

[Alexey Gaidukov]

The following example doesn't work on XForms with the patch. Example works well on XForms 0.8.7pre.

http://xforms-examples.googlecode.com/svn/trunk/02-controls/07-Range/range-bind.xhtml

Error: XForms Error (8): id (input-one-bind) does not refer to a bind element
Source File: http://xforms-examples.googlecode.com/svn/trunk/02-controls/07-Range/range-bind.xhtml
Line: 0
Source Code:
<xf:output bind="input-one-bind"/>

Comment 20

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

Sergey, have you figured out why http://xforms-examples.googlecode.com/svn/trunk/02-controls/07-Range/range-bind.xhtml doesn't work?

(Note, I haven't tested the patch locally.)

Comment 21

[Sergey Reymerov]

Attachment: Full XForms patch

Unfortunatly I made more when adopt XForms, but it is in another patch. 
https://bugzilla.mozilla.org/show_bug.cgi?id=521246
https://bugzilla.mozilla.org/show_bug.cgi?id=525735

This example is worklable on my XForms version.

Comment 22

[Sergey Reymerov]

I tested it on 3.7. 
It really doesn't work. 
I take error:

Error: XForms Error (8): id (input-one-bind) does not refer to a bind element
Source File: http://xforms-examples.googlecode.com/svn/trunk/02-controls/07-Range/range-bind.xhtml
Line: 0
Source Code:
<xf:range bind="input-one-bind" start="1" end="5" step="1" incremental="true"/>

Comment 23

[Sergey Reymerov]

In my opinion problem is in bind-element. I discovered that when range-element bind to the bind-element GetElementByContextId returns null. It mean that bind-element are not in xmlDoc. Now I look at nsXFormsModelElement::ProcessBind.

Comment 24

[Alexey Gaidukov]

Attachment: Correct example

I found the problem. The example from google is incorrect. Tag style is in wrong place. 

So patch in patch there is no problems in bindings.

Comment 25

[Alexey Gaidukov]

(In reply to comment #24)
> So patch in patch there is no problems in bindings.
So in patch there is no problems in bindings.

Comment 26

[Sergey Reymerov]

Olli, so it's not the problem of my patch. And it's wonderful. 
Well we can continue review that patch.

Comment 27

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

Ok, I'll review this soonish. Sorry for the delay.

Comment 28

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

Comment on attachment 409594 [details] [diff] [review]
Schema validation patch

Seems like nsCStringArray was modified in Bug 466622.
Could you perhaps use ParseString (defined in nsStringAPI.h) and nsTArray<nsCString>  ?

Comment 29

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

Comment on attachment 409594 [details] [diff] [review]
Schema validation patch

Maybe the new API isn't too useful after all.

>diff -r 27882682ae1a src/nsSchemaValidator.cpp
>--- a/src/nsSchemaValidator.cpp	Wed Sep 16 08:50:04 2009 +0800
>+++ b/src/nsSchemaValidator.cpp	Sun Nov 01 20:36:03 2009 +0800
>@@ -1455,7 +1455,18 @@
> 
>   if (listSimpleType) {
>     nsCStringArray stringArray;
>-    stringArray.ParseString(NS_ConvertUTF16toUTF8(aNodeValue).get(), " \t\r\n");
>+    char *rest = strdup(NS_ConvertUTF16toUTF8(aNodeValue).get());
>+    char *newStr = rest;
>+    char *token = NS_strtok(" \t\r\n", &newStr);
>+    while (token) {
>+      if (*token) {
>+        /* calling AppendElement(void*) to avoid extra nsCString copy */
I don't understand this comment. You're calling AppendCString, not AppendElement.

Fix that, r=me

Comment 30

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

Or, if the new API works well in this case, please use that. (and ask new review)

Comment 31

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

Comment on attachment 409598 [details] [diff] [review]
XForms patch

>diff -r 3478e987965d nsXFormsControlStub.cpp
>--- a/nsXFormsControlStub.cpp	Thu Sep 10 11:15:57 2009 +0800
>+++ b/nsXFormsControlStub.cpp	Mon Nov 02 00:46:03 2009 +0800
>@@ -55,6 +55,8 @@
> #include "nsIContent.h"
> #include "nsIDOM3Node.h"
> #include "nsIDOMAttr.h"
>+#include "nsFocusManager.h"
>+#include "nsServiceManagerUtils.h"
> 
> /** This class is used to generate xforms-hint and xforms-help events.*/
> class nsXFormsHintHelpListener : public nsIDOMEventListener {
>@@ -586,13 +588,22 @@
>       // guaranteed that the focus controller outlives us, so it
>       // is safe to hold on to it (since we can't die until it has
>       // died).
>-      nsIFocusController *focusController =
>-        doc->GetWindow()->GetRootFocusController();
>-      if (focusController &&
>-          type.EqualsLiteral(sXFormsEventsEntries[eEvent_Next].name)) {
>-        focusController->MoveFocus(PR_TRUE, nsnull);
>+      nsCOMPtr<nsIFocusManager> focusManager = do_GetService(FOCUSMANAGER_CONTRACTID);
>+      if (!focusManager) 
>+        return NS_OK;
>+      if (focusManager && type.EqualsLiteral(sXFormsEventsEntries[eEvent_Next].name)) {
>+        focusManager->MoveFocus(doc->GetWindow(), 
>+                                static_cast<nsIDOMElement *>(nsnull), 
>+                                static_cast<PRUint32>(nsIFocusManager::MOVEFOCUS_FORWARD), 
>+                                static_cast<PRUint32>(0), 
>+                                static_cast<nsIDOMElement **>(nsnull));
You don't need these static_casts here.


>+        
>       } else {
>-        focusController->MoveFocus(PR_FALSE, nsnull);
>+        focusManager->MoveFocus(doc->GetWindow(), 
>+                                static_cast<nsIDOMElement *>(nsnull), 
>+                                static_cast<PRUint32>(nsIFocusManager::MOVEFOCUS_BACKWARD), 
>+                                static_cast<PRUint32>(0), 
>+                                static_cast<nsIDOMElement **>(nsnull)); 
Same this here.

       
>+++ b/nsXFormsMessageElement.cpp	Mon Nov 02 00:46:03 2009 +0800
>@@ -630,11 +630,15 @@
>   aEvent->GetTarget(getter_AddRefs(target));
>   nsCOMPtr<nsIDOMElement> targetEl(do_QueryInterface(target));
>   if (targetEl) {
>-    nsCOMPtr<nsIDOMNSDocument> nsDoc(do_QueryInterface(aDoc));
>+    nsCOMPtr<nsIDocument> nsDoc(do_QueryInterface(aDoc));
>     if (nsDoc) {
>       PRInt32 oldX = mPosX;
>       PRInt32 oldY = mPosY;
>       nsCOMPtr<nsIBoxObject> box;
>+      
>+      if(!nsDoc)
>+        return NS_ERROR_UNEXPECTED;
>+
>       nsDoc->GetBoxObjectFor(targetEl, getter_AddRefs(box));

The method has nsCOMPtr<nsIDocument> doc variable. Just use that.

> 
>     nsCStringArray schemas;
>-    schemas.ParseString(NS_ConvertUTF16toUTF8(schemaList).get(), " \t\r\n");
>+
>+    char *rest = strdup(NS_ConvertUTF16toUTF8(schemaList).get());
>+    char *newStr = rest;
>+    char *token = NS_strtok(" \t\r\n", &newStr);
>+    while (token) {
>+      if (*token) {
>+        /* calling AppendElement(void*) to avoid extra nsCString copy */
Again, this comment talks about AppendElement which you aren't using

>@@ -352,7 +355,9 @@
>     // Flush layout before moving focus. Otherwise focus controller might
>     // (re)focus something in the deselected <case>.
>     doc->FlushPendingNotifications(Flush_Layout);
>-    focusController->MoveFocus(PR_TRUE, mElement);
>+      focusManager->MoveFocus(doc->GetWindow(), static_cast<nsIDOMElement *>(nsnull), 
>+                              static_cast<PRUint32>(nsIFocusManager::MOVEFOCUS_FORWARD), static_cast<PRUint32>(0), 
>+                              &mElement);

This looks wrong. mElement will contain the newly focused element, but nothing releases the
old value - so this leaks. And I think you don't actually want to pass mElement as the
last parameter but as the second. Or just use focusManager->SetFocus(mElement, ...)

Comment 32

[Sergey Reymerov]

Attachment: XForms patch 2

I've made new patch with most every problem that you specify. 
What I din't change?!

>>+++ b/nsXFormsMessageElement.cpp	Mon Nov 02 00:46:03 2009 +0800
>>@@ -630,11 +630,15 @@
>>   aEvent->GetTarget(getter_AddRefs(target));
>>   nsCOMPtr<nsIDOMElement> targetEl(do_QueryInterface(target));
>>   if (targetEl) {
>>-    nsCOMPtr<nsIDOMNSDocument> nsDoc(do_QueryInterface(aDoc));
>>+    nsCOMPtr<nsIDocument> nsDoc(do_QueryInterface(aDoc));
>>     if (nsDoc) {
>>       PRInt32 oldX = mPosX;
>>       PRInt32 oldY = mPosY;
>>       nsCOMPtr<nsIBoxObject> box;
>>+      
>>+      if(!nsDoc)
>>+        return NS_ERROR_UNEXPECTED;
>>+
>>       nsDoc->GetBoxObjectFor(targetEl, getter_AddRefs(box));
>
>The method has nsCOMPtr<nsIDocument> doc variable. Just use that.
doc variable is in another scope.

About ParseString. I think that ParseString from nsStringAPI.h isn't useful. And I just remove foolish cooment about AppendElement.

Comment 33

[Sergey Reymerov]

Incidentally, who can also review this patches?

Comment 34

[alexander :surkov (:asurkov)]

Comment on attachment 415626 [details] [diff] [review]
XForms patch 2


>     nsCStringArray schemas;
>-    schemas.ParseString(NS_ConvertUTF16toUTF8(schemaList).get(), " \t\r\n");
>+
>+    char *rest = strdup(NS_ConvertUTF16toUTF8(schemaList).get());
>+    char *newStr = rest;
>+    char *token = NS_strtok(" \t\r\n", &newStr);
>+    while (token) {
>+      if (*token) {
>+        nsCString schema(token);
>+        schemas.AppendCString(schema);
>+      }
>+      token = NS_strtok(" \t\r\n", &newStr);
>+    }
>+    free(rest);

I think you don't need an array. Just put all schema processing logic inside of this loop.

Comment 35

[Sergey Reymerov]

Attachment: XForms patch 3

I've change patch according to Alex remark.

Comment 36

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

Comment on attachment 417050 [details] [diff] [review]
XForms patch 3


>+    char *schemaRest = strdup(NS_ConvertUTF16toUTF8(schemaList).get());
>+    char *newSchemaStr = schemaRest;
>+    char *nextSchemaToken = NS_strtok(" \t\r\n", &newSchemaStr);    
>+    while (nextSchemaToken) {
>+	    if (*nextSchemaToken) {
>+     	  mSchemaTotal++;	    
>+        rv = NS_OK;
>+        nsCAutoString uriSpec;
>+        nsCOMPtr<nsIURI> newURI;
>+        NS_NewURI(getter_AddRefs(newURI), nsCString(nextSchemaToken), nsnull, baseURI);
>+        nsCOMPtr<nsIURL> newURL = do_QueryInterface(newURI);
Seems like there are some tab characters here. Could you upload a patch where
only spaces are used. It is hard to review the current one.
And use 2 space indentation.

Comment 37

[Sergey Reymerov]

Attachment: XForms patch 3

I've change '\t' symbols replace by ' ' symbol. Also I found unnecessary change in nsXFormsSelect.cpp.

Comment 38

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

Comment on attachment 417918 [details] [diff] [review]
XForms patch 3

>@@ -2069,7 +2069,7 @@ nsXFormsUtils::GetElementByContextId(nsI
> 
>   nsresult rv = document->GetElementById(aId, aElement);
>   NS_ENSURE_SUCCESS(rv, rv);
>-
>+ 
No reason for this change.

Comment 39

[alexander :surkov (:asurkov)]

Comment on attachment 417918 [details] [diff] [review]
XForms patch 3


>+      nsCOMPtr<nsIFocusManager> focusManager = do_GetService(FOCUSMANAGER_CONTRACTID);
>+      if (!focusManager) 
>+        return NS_OK;
>+      if (focusManager && type.EqualsLiteral(sXFormsEventsEntries[eEvent_Next].name)) {

you don't need to check focusManager inside of 'if' statemenet.

>+NS_IMETHODIMP
>+nsXFormsDOMEvent::GetPreventDefault(PRBool* aReturn)
>+{
>+  return NS_ERROR_NOT_IMPLEMENTED;
>+}

do you really need this? because it is implemented by NS_FORWARD_NSIDOMEVENT(mInner->). Why should it be unimplemented?

>-    nsCOMPtr<nsIDOMNSDocument> nsDoc(do_QueryInterface(aDoc));
>+    nsCOMPtr<nsIDocument> nsDoc(do_QueryInterface(aDoc));
>     if (nsDoc) {
>       PRInt32 oldX = mPosX;
>       PRInt32 oldY = mPosY;
>       nsCOMPtr<nsIBoxObject> box;
>+      
>+      if(!nsDoc)
>+        return NS_ERROR_UNEXPECTED;

how can it happen because nsDoc is not null already?

>+        NS_NewURI(getter_AddRefs(newURI), nsCString(nextSchemaToken), nsnull, baseURI);

can you use nsDependentCString here?

r=me with these fixed/addressed

Comment 40

[alexander :surkov (:asurkov)]

Comment on attachment 409594 [details] [diff] [review]
Schema validation patch


>+    char *rest = strdup(NS_ConvertUTF16toUTF8(aNodeValue).get());
>+    char *newStr = rest;
>+    char *token = NS_strtok(" \t\r\n", &newStr);
>+    while (token) {
>+      if (*token) {
>+        /* calling AppendElement(void*) to avoid extra nsCString copy */

please fix the comment or the code

>+        nsCString schema(token);
>+        stringArray.AppendCString(schema);
>+      }
>+      token = NS_strtok(" \t\r\n", &newStr);
>+    }
>+    free(rest);
> 
>     PRUint32 count = stringArray.Count();
> 

r=me with fixed

Comment 41

[Sergey Reymerov]

Attachment: schema-validation patch

Comment 42

[Sergey Reymerov]

Attachment: XForms patch

I've change every remark except this:
>+NS_IMETHODIMP
>+nsXFormsDOMEvent::GetPreventDefault(PRBool* aReturn)
>+{
>+  return NS_ERROR_NOT_IMPLEMENTED;
>+}

Comment 43

[alexander :surkov (:asurkov)]

(In reply to comment #42)
> Created an attachment (id=419119) [details]
> XForms patch
> 
> I've change every remark except this:
> >+NS_IMETHODIMP
> >+nsXFormsDOMEvent::GetPreventDefault(PRBool* aReturn)
> >+{
> >+  return NS_ERROR_NOT_IMPLEMENTED;
> >+}

What is the reason?

Comment 44

[alexander :surkov (:asurkov)]

Comment on attachment 419119 [details] [diff] [review]
XForms patch


>-      nsIFocusController *focusController =
>-        doc->GetWindow()->GetRootFocusController();
>-      if (focusController &&
>-          type.EqualsLiteral(sXFormsEventsEntries[eEvent_Next].name)) {
>-        focusController->MoveFocus(PR_TRUE, nsnull);
>+      nsCOMPtr<nsIFocusManager> focusManager = do_GetService(FOCUSMANAGER_CONTRACTID);
>+      if (focusManager && type.EqualsLiteral(sXFormsEventsEntries[eEvent_Next].name)) {
>+        focusManager->MoveFocus(doc->GetWindow(), nsnull, nsIFocusManager::MOVEFOCUS_FORWARD, 0, nsnull);
>+        
>       } else {
>-        focusController->MoveFocus(PR_FALSE, nsnull);
>+        focusManager->MoveFocus(doc->GetWindow(), nsnull, nsIFocusManager::MOVEFOCUS_BACKWARD, 0, nsnull);
>       }

I know this way is how it was but if focusManger might be a null then this code will crash.

>--- a/nsXFormsSelectElement.cpp	Thu Sep 10 11:15:57 2009 +0800
>+++ b/nsXFormsSelectElement.cpp	Thu Dec 24 22:35:12 2009 +0800
>@@ -66,6 +66,8 @@ public:
>   NS_IMETHOD ChildInserted(nsIDOMNode *aChild, PRUint32 aIndex);
>   NS_IMETHOD ChildAppended(nsIDOMNode *aChild);
>   NS_IMETHOD ChildRemoved(PRUint32 aIndex);
>+  NS_IMETHOD BeginAddingChildren();
>+  NS_IMETHOD DoneAddingChildren();

That might be necessary to make xforms working on mozilla central but this code is not pretty obvious so I would like to get some objections and comments in the code. As well I think changes like these should result in rerequest review from Olli additionally.

Comment 45

[Sergey Reymerov]

Attachment: XForms patch

I've removed changes in nsXFormsSelect.cpp. It's necessary.

Comment 46

[alexander :surkov (:asurkov)]

Comment on attachment 419284 [details] [diff] [review]
XForms patch

>diff -r 3478e987965d nsXFormsControlStub.cpp
>--- a/nsXFormsControlStub.cpp	Thu Sep 10 11:15:57 2009 +0800
>+++ b/nsXFormsControlStub.cpp	Mon Dec 28 17:59:37 2009 +0800
>@@ -55,6 +55,8 @@
> #include "nsIContent.h"
> #include "nsIDOM3Node.h"
> #include "nsIDOMAttr.h"
>+#include "nsFocusManager.h"
>+#include "nsServiceManagerUtils.h"
> 
> /** This class is used to generate xforms-hint and xforms-help events.*/
> class nsXFormsHintHelpListener : public nsIDOMEventListener {
>@@ -586,13 +588,14 @@ nsXFormsControlStub::HandleDefault(nsIDO
>       // guaranteed that the focus controller outlives us, so it
>       // is safe to hold on to it (since we can't die until it has
>       // died).
>-      nsIFocusController *focusController =
>-        doc->GetWindow()->GetRootFocusController();
>-      if (focusController &&
>-          type.EqualsLiteral(sXFormsEventsEntries[eEvent_Next].name)) {
>-        focusController->MoveFocus(PR_TRUE, nsnull);
>+      nsCOMPtr<nsIFocusManager> focusManager = do_GetService(FOCUSMANAGER_CONTRACTID);
>+      if (!focusManager)
>+		    return NS_OK;

nit: fix indent, it should be two spaces.

>+      if (focusManager && type.EqualsLiteral(sXFormsEventsEntries[eEvent_Next].name)) {
>+        focusManager->MoveFocus(doc->GetWindow(), nsnull, nsIFocusManager::MOVEFOCUS_FORWARD, 0, nsnull);

again, you don't need to test focusManager twice.

>--- a/nsXFormsMessageElement.cpp	Thu Sep 10 11:15:57 2009 +0800
>+++ b/nsXFormsMessageElement.cpp	Mon Dec 28 17:59:37 2009 +0800
>@@ -630,7 +630,7 @@ nsXFormsMessageElement::HandleEphemeralM
>   aEvent->GetTarget(getter_AddRefs(target));
>   nsCOMPtr<nsIDOMElement> targetEl(do_QueryInterface(target));
>   if (targetEl) {
>-    nsCOMPtr<nsIDOMNSDocument> nsDoc(do_QueryInterface(aDoc));
>+    nsCOMPtr<nsIDocument> nsDoc(do_QueryInterface(aDoc));

why is this change?

>@@ -352,7 +355,7 @@ nsXFormsSwitchElement::SetFocus(nsIDOMEl
>     // Flush layout before moving focus. Otherwise focus controller might
>     // (re)focus something in the deselected <case>.
>     doc->FlushPendingNotifications(Flush_Layout);
>-    focusController->MoveFocus(PR_TRUE, mElement);
>+      focusManager->SetFocus(mElement, 0);

nit: check indent

cancelling review, I would like to look at new patch.

Comment 47

[Sergey Reymerov]

Attachment: XForms patch

I change every remark. 
Also I change back nsIDocument to nsIDOMNSDocument. I don't remember why I did so. But think that mozilla couldn't compile in past with (nsIDOMNSDocument). And now it can.

Comment 48

[alexander :surkov (:asurkov)]

Attachment: xforms patch

few minor changes to make compile a patch.

Comment 49

[alexander :surkov (:asurkov)]

xforms patch landed - http://hg.mozilla.org/xforms/rev/c54025ed6b56
schema-validation patch landed - http://hg.mozilla.org/schema-validation/rev/f22c99b37a1b

Comment 50

[Philipp Wagner [:imphil]]

Something is not yet right with these patches. 

* go to http://www.mozilla.org/projects/xforms/samples/insurance_form/app_certificates.xhtml
* check "I agree"
* now the other "tabs" on top ("name and address", ...) should appear
* uncheck it again and check it again -> now it works.

The same behavior can be seen on the "Evidence of Name" tab in the same form.

Comment 51

[alexander :surkov (:asurkov)]

I think it's worth to file following up bug for this and somebody should look at the problem. Sergey, could you as the patch author?

Comment 52

[Sergey Reymerov]

Yes, I can.

Comment 53

[Sergey Reymerov]

I submit new bug 537881.