265467 - Integrate the rest of the MDG code

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Allan Beaufour]

We still need to further integrate the MDG with the rest of the code. The MDG needs
* to be informed of changed instance data
* to send the proper events for changed data

Comment 1

[Allan Beaufour]

Attachment: Current status of integration

The code is far from "check in-ready", but you can see where I'm heading, and
any comments are appreciated.

The status is:
* Tied single node bindings in form controls to the MDG (nodeset bindings are
not handled yet)
  - Comments on how to do tie bound nodes and controls together?

* Fixed misc. bugs

* Changed some things in nsXFormsMDGEngine to make the code easier to
understand:
  - Created nsXFormsNodeState to handle the flags

  - Changed misc. variable names

* Styling (http://www.w3.org/TR/xforms/sliceF.html)
  - We need to handle the proposed pseudo classes and elements

  - We need to use the above, and supply a default style sheet instead of
    style directly in the code

  - Is it correct that CSS |input {}| should match the inner <html:input> field
on <xforms:input>? I would say no, we need to define that as ::value somehow.

  - I will start to look into the pseudo-classes, but it needs patching in a
lot of core Mozilla code (nsCSSStyleSheet, nsIEventStateManager, and
others...). For the moment I have implemented it via attributes so that you
(and I) can see it in action.

Comment 2

[Allan Beaufour]

Attachment: Small test form

Here's a form that uses the patch, where you can see the MDG in action and also
see the CSS-styling I'm talking about.

Comment 3

[Allan Beaufour]

Attachment: Updated to current CVS

Comment 4

[Allan Beaufour]

Attachment: Updated to current CVS

Comment 5

[Allan Beaufour]

Attachment: Updated to current CVS

Comment 6

[Allan Beaufour]

Attachment: Added some comments, etc.

Comment 7

[Allan Beaufour]

I promised to do a better description of my patch, so here it goes. The key
feature of this patch is:

Enabling the controls to react to changes to the instance data that they are
bound to. The MDG engine returns a list of changed nodes, and the model figures
out which controls are bound to these nodes, and sends the appropriate events to
them.

Code-wise this means:
1) Changing nsIModelElementPrivate::addFormControl()

So controls also can inform the model of which node they are bound to.

Note that the "node part" is only a temporary hack, the model should be informed
about the XPath expression the control is bound to, not only the node, so all
dependencies can be watched.

2) Changing nsXFormsModelElement::mFormControls

So it is a hashmap, so the node that they are bound to can be stored too. This
needs to be changed further, to accomodate all dependencies for a control.

3) Implementing more of nsXFormsModelElement

Recalculate() and Revalidate() have grown a bit, so they handle the nodes that
have changed.

4) Adding nsIXFormsControl::getElement()

To get hold of the HTML element used by the control, as I am dispatching the
events to this. This is not the right approach, but I could not seem to get
dispatching directly to the controls to work :(

5) Added a nsXFormsUtils::HandleEvent

This handles |xforms-*| received by the controls. As we are in need of the CSS 3
UI pseudo-classes (see bug 271720), I set attributes on the controls instead of
classes, so it is "CSS-stylable". (see the "Small test form" this bug, for an
example)

The rest of the changes to nsXFormsUtils is part of bug 265931, ignore those. 

6) Changed input and output to use the above

Input and output handles the events, and I've refactored the code into binding
code and refreshing code. I'm not 100% done with that yet I think. More thinking
to do.

As a "bonus", I've also changed nsXFormsMDGEngine a bit:
B1) Implemented nsXFormsNodeState

I've moved the state reprensentation of a node (is it readonly, relevant, etc.)
out from nsXFormsMDGEngine and into a seperate class. It is quite simple, just
bit-checking, but now it's easier to handle.

B2) Improved style

Changed some variable names, and code styling to make the code more easy to
understand and conform better to Mozilla style.

B3) Fixed a bug or two

Comment 8

[aaronr]

Ok, looked through the code.  Finally.  Sorry that it took so long.  I don't
know if it all has sunk in, yet, but here are my initial thoughts.

Are the todo's in the code meant for future patches or yet to be completed items
on this patch?

By introducing Bind and the mBoundNode, you are changing the behavior slightly
in that the bound node is only determined once and then if the node's parent
changes or a binding attribute changes instead of on every Refresh().  The
obvious (potential) holes that I see is if a bind changes its nodeset or the
instance document changes and that mBoundNode no longer exists.  Will the
control know to figure out its mBoundNode again?  Edge case, to be sure.

Comments above make it sound like GetElement is supposed to return the HTML
element of the control.  But GetElement is returning the xforms element.  Could
you please clarify?  And make sure comments in nsIXFormsControl.idl file a
little clearer on what you mean by element and what you mean by control?

Typo in nsXFormsMDGEngine.cpp.. you have "#// define DEBUG_XF_MDG" right above
where you define gMIPNames.

In nsXFormsMDGEngine::GetNode you have the test aType!=eModel_type as part of
the while loop.  Testing every time, but the result will never change.

I'm wondering why in nsXFormsNodeState.cpp in IsValid() you are testing for
eFlag_CONSTRAINT and not eFlag_VALID.  Not really intuitive why, esp. since the
validity checking stuff isn't in, yet, so tough to puzzle out myself.  Might
need to clarify with a comment.

In nsXFormsOutputElement, you seem to ignore @value if the @ref is set.  Which
is according to spec.  But shouldn't you also ignore @value if @bind is set? 
But before you get hasty with this, please look at the Errata in E0.  It says
that the context node of the @value evaluation should be the bound node.  Which
seems to me to take into account @ref or @bind, right?  Could you please get
David to clarify?

All in all the changes make good sense to me.  Sorry, I don't know enough about
CSS to comment on any of that.

Comment 9

[Allan Beaufour]

(In reply to comment #8)
> Ok, looked through the code.  Finally.  Sorry that it took so long.

Well, I'm also a bit to blame myself, it's not exactly a small patch... and it's
getting bigger and bigger :(

> Are the todo's in the code meant for future patches or yet to be completed
> items on this patch?

A nice mixture of both. The priority only exists in my head, and you do not want
to go there :)

> By introducing Bind and the mBoundNode, you are changing the behavior slightly
> in that the bound node is only determined once and then if the node's parent
> changes or a binding attribute changes instead of on every Refresh().  The
> obvious (potential) holes that I see is if a bind changes its nodeset or the
> instance document changes and that mBoundNode no longer exists.  Will the
> control know to figure out its mBoundNode again?  Edge case, to be sure.

Yes, but a good point! I'll deliver a new (and bigger, *sigh*) patch in a day or
two, where it should be easier to see when and how mBoundNode is updated. My
assumption is that bind's bindings are static, and if they are updated then you
need to do a xforms-rebuild. This may, or may not, be in accordance with the
spec... All other bindings should be updated automatically by the code.

> I'm wondering why in nsXFormsNodeState.cpp in IsValid() you are testing for
> eFlag_CONSTRAINT and not eFlag_VALID.  Not really intuitive why, esp. since the
> validity checking stuff isn't in, yet, so tough to puzzle out myself.  Might
> need to clarify with a comment.

Constraints determine whether a node is valid or not. eFlag_VALID is now gone to
meet it's maker, I do not know what it was doing there.

> In nsXFormsOutputElement, you seem to ignore @value if the @ref is set.  Which
> is according to spec.  But shouldn't you also ignore @value if @bind is set?

Sure I do! Thanks.
 
> But before you get hasty with this, please look at the Errata in E0.  It says
> that the context node of the @value evaluation should be the bound node.
> Which seems to me to take into account @ref or @bind, right? Could you please
> get David to clarify?

David is investigating it. It makes no sense in my eyes.

I also agree with the rest of the comments, and they'll be included in the next
patch.

Thanks you for the comments.

Comment 10

[Doron Rosenberg (IBM)]

+ /// @todo Schema validity should be checked here (XXX)

Is this the only place schema validation needs to be checked?

Comment 11

[Allan Beaufour]

(In reply to comment #10)
> + /// @todo Schema validity should be checked here (XXX)
> 
> Is this the only place schema validation needs to be checked?

AFAIK, yes. But frankly speaking, I haven't really looked into the
schema-support. We could try hooking it up here for a start :)

Comment 12

[Allan Beaufour]

Attachment: Newest edition, for the XForms branch

The patch from hell. Keeps growing, sorry for that. I could _really_ use some
comments... Now it's pretty mature, and works on the new branch and on all
controls.

Big lines:
----------
* Extended nsIXFormsControl to include attributes for getting the bound node
and the dependency nodes. I have thus placed the responsibility of keeping
track of dependencies on the controls. The model asks the controls for any
dependencies through nsIXFormsControl, when it revalidates.

* Implemented nsXFormsControlStub, and let all controls with node bindings
inherit from that instead of directly from nsIXFormsControl. It also inherits
from nsXFormsXMLVisualStub, so controls only need to inherit from
nsXFormsControlStub.

  - nsXFormsControlStub::ProcessNodeBinding() should be used by controls
instead of nsXFormsUtils::EvaluateNodeBinding()

  - nsXFormsControlStub::GetReadonlyState() gets the controls current readonly
state

* Fixed nsXFormsGroupElement's problems with mDoneAddingChildren, etc. The
solution was simple: Hook up on ParentChanged() as everybody else.

* Added @model to attributes watched for changes by controls

* Fixes bug 277475, /me hums "I did it my way" :)

These (at least) should probably be split to seperate bugs...
-------------------------------------------------------------
* Repeat was wathing @ref, it should watch @nodeset

* It seems like not everything from the nested bind patch made it to the
code... this patch includes fix for inner bind handling. (It moves the inner
bind handling inside the for loop.)

Problems:
---------
* I mess up Aaron's XPath functions (bug 258472). My guess is that its because
the changes to the instance data happens behind the MDG's back? If so, it
should be easy to fix, but enough for today.

* The attibutes (@readonly, @relevant, etc.) are not set correctly.


Phew... There are still some loose ends, but its close to being finished.

[It has taken me quite some time to get the patch ready for the branch... so I
might have missed a thing or two in the cut-n-paste frenzy I've been doing all
morning :(]

Comment 13

[Allan Beaufour]

(In reply to comment #12)
> * I mess up Aaron's XPath functions (bug 258472). My guess is that its because
> the changes to the instance data happens behind the MDG's back? If so, it
> should be easy to fix, but enough for today.

I was using Aaron's test case, which was why I thought the bug was in the XPath
functions. Stupid me, it's probably not, but in <setvalue/> which needs to
inform the MDG. Should be an easy fix.

Comment 14

[Brian Ryner (not reading)]

Comment on attachment 170929 [details] [diff] [review]
Newest edition, for the XForms branch

--- xforms.newbranch/nsIXFormsControl.idl	2004-11-04 22:12:22.000000000
+0100
+++ xforms.mdg-branch/nsIXFormsControl.idl	2005-01-10 16:56:24.000000000
+0100
@@ -33,20 +33,49 @@
...
+  /**
+   * The instance nodes that the control depend on. These MUST be sorted,
+   * pointer-order, ascending.
+   */
+  readonly attribute nsIArray dependencies;

Would be nice if this could be a nsCOMArray since this method doesn't need to
be scriptable.	It should be possible to declare it as such in IDL using
[native, ref] or something like that... there should be some examples in the
tree.

--- xforms.newbranch/nsXFormsContextContainer.cpp	2005-01-06
21:18:06.000000000 +0100
+++ xforms.mdg-branch/nsXFormsContextContainer.cpp	2005-01-11
16:08:09.296271136 +0100
>-// nsXFormsControl
>+// nsIXFormsControl
> nsresult
> nsXFormsContextContainer::Refresh()
> {
>   return NS_OK;
> }

Maybe this no-op implementation could be part of nsXFormsControlStub?

I know I'm not very far through this at all, but I need to move on to some
other work.  I'll pick back up at nsXFormsControlStub::ProcessNodeBinding.

Comment 15

[Allan Beaufour]

Attachment: Misc. fixes

Changes:
* Aparently the patch was not totally up-to-date with the branch. This one is.

* Setvalue-error is fixed

* Incorporates nsXFormsSwitchElement into new structure, I must have forgotten
in the first place

* Fixes two Windows compile problems that Aaron found

* Fixes an error check on the wrong variable in nsXFormsGroupElement

Hopefully this works. It's done a bit in the blind as I do not have the branch
checked out at this location :(

Comment 16

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

Comment on attachment 170962 [details] [diff] [review]
Misc. fixes

Few comments

>+  readonly attribute nsXFormsMDGEngine MDG;

Hmm, using non-interface class in an interface.


>+class nsXFormsContextContainer : public nsXFormsControlStub,
>                                   public nsIXFormsContextControl

Indentation


>+NS_IMETHODIMP
>+nsXFormsControlStub::DocumentChanged(nsIDOMDocument *aNewDocument)
>+{
>+  // We need to re-evaluate our instance data binding when our document
>+  // changes, since our context can change
>+  if (aNewDocument) {
>+    Bind();
>+    Refresh();
>+  }

Shouldn't you set mMDG = nsnull somewhere in this method


>@@ -326,94 +242,86 @@ nsXFormsInputElement::Blur(nsIDOMEvent *
>       PRBool checked;
>       input->GetChecked(&checked);
>       value.AssignASCII(checked ? "1" : "0", 1);
>     } else {
>       input->GetValue(value);
>     }
>   }
> 
>-  nsXFormsUtils::SetNodeValue(singleNode, value);
>+  PRBool changed;
>+  nsresult rv = mMDG->SetNodeValue(mBoundNode, value, PR_TRUE, &changed);

what if someone has used scripts to modify instance data, for example to
move the node to a different place. This is then modifying wrong node, isn't
it(?)
Or can we assume that rebuild() should have been called?


>       if (g->HasExpr()) {
>-        rv = ComputeMIPWithInheritance(MDG_FLAG_READONLY, MDG_FLAG_DISPATCH_READONLY_CHANGED, MDG_FLAG_INHERITED_READONLY, g, changedNodes);
>+        rv = ComputeMIPWithInheritance(eFlag_READONLY, eFlag_DISPATCH_READONLY_CHANGED, eFlag_INHERITED_READONLY, g, aChangedNodes);

Line length

>-        rv = ComputeMIPWithInheritance(MDG_FLAG_RELEVANT, MDG_FLAG_DISPATCH_RELEVANT_CHANGED, MDG_FLAG_INHERITED_RELEVANT, g, changedNodes);
>+        rv = ComputeMIPWithInheritance(eFlag_RELEVANT, eFlag_DISPATCH_RELEVANT_CHANGED, eFlag_INHERITED_RELEVANT, g, aChangedNodes);

Line length


>+  /// @todo Assuming that moving element x, will delete x, and move list "one to the left",
>+  /// ie. x = x+1, x+1=x+2, ... etc.
>+  /// Is that correct? (XXX)

http://lxr.mozilla.org/seamonkey/source/xpcom/ds/nsVoidArray.cpp#573


>+    if (ns->ShouldDispatchValid()) {
>+      if (ns->IsValid()) {
>+        nsXFormsUtils::DispatchEvent(element, eEvent_Valid);
>+      } else {
>+        nsXFormsUtils::DispatchEvent(element, eEvent_Invalid);
>+      }

Perhaps nsXFormsUtils::DispatchEvent(element, ns->IsValid() ? 
				     eEvent_Valid : eEvent_Invalid)


>+    }
>+    if (ns->ShouldDispatchReadonly()) {
>+      if (ns->IsReadonly()) {
>+        nsXFormsUtils::DispatchEvent(element, eEvent_Readonly);
>+      } else {
>+        nsXFormsUtils::DispatchEvent(element, eEvent_Readwrite);
>+      }

Same here

>+    }
>+    if (ns->ShouldDispatchRequired()) {
>+      if (ns->IsRequired()) {
>+        nsXFormsUtils::DispatchEvent(element, eEvent_Required);
>+      } else {
>+        nsXFormsUtils::DispatchEvent(element, eEvent_Optional);
>+      }

And here


>+    }
>+    if (ns->ShouldDispatchRelevant()) {
>+      if (ns->IsRelevant()) {
>+        nsXFormsUtils::DispatchEvent(element, eEvent_Enabled);
>+      } else {
>+        nsXFormsUtils::DispatchEvent(element, eEvent_Disabled);
>+      }

And here

Comment 17

[aaronr]

I still get a couple of conflicts when I apply the patch, but the 2005-01-11
13:37 PST patch builds and works for me

Comment 18

[Allan Beaufour]

(In reply to comment #16)
> (From update of attachment 170962 [details] [diff] [review] [edit])
> >+  readonly attribute nsXFormsMDGEngine MDG;
> 
> Hmm, using non-interface class in an interface.

:( I guess that's a no-no? I'll have to create interfaces for the public
functions of the MDG then, and also nsXFormsNodeState. Correct?
 
> >+class nsXFormsContextContainer : public nsXFormsControlStub,
> >                                   public nsIXFormsContextControl
> 
> Indentation

On purpose, I did not change misc. indentations that were wrong in the first
place.... my patch was big enough. Now it's even bigger...

> >@@ -326,94 +242,86 @@ nsXFormsInputElement::Blur(nsIDOMEvent *
> >       PRBool checked;
> >       input->GetChecked(&checked);
> >       value.AssignASCII(checked ? "1" : "0", 1);
> >     } else {
> >       input->GetValue(value);
> >     }
> >   }
> > 
> >-  nsXFormsUtils::SetNodeValue(singleNode, value);
> >+  PRBool changed;
> >+  nsresult rv = mMDG->SetNodeValue(mBoundNode, value, PR_TRUE, &changed);
> 
> what if someone has used scripts to modify instance data, for example to
> move the node to a different place. This is then modifying wrong node, isn't
> it(?)
> Or can we assume that rebuild() should have been called?

The spec. is not exactly clear afaik on dynamic changes. I expect people to do a
rebuild if they mess with instance data through "unauthorized ways". A patch
would be for the MDG/Model to listen for mutation events, but that was shot down
due to speed during a discussion last year.

Agree with the rest. I have cut down line length on entire
nsXFormsMDGEngine.[h|cpp].

Comment 19

[Allan Beaufour]

(In reply to comment #14)

I agree with your comments Brian, but could we wait with that. I'm getting
rather tired (understatement) of updating to current CVS every other day. Could
we get this bug in, so I could rid of this 200K+ monster?

Comment 20

[Allan Beaufour]

Attachment: Revised

Includes Olli's comments (excludes setvalue.diff :) )

Comment 21

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

Comment on attachment 171034 [details] [diff] [review]
Revised

This doesn't refresh controls properly.

Comment 22

[Allan Beaufour]

(In reply to comment #21)
> (From update of attachment 171034 [details] [diff] [review] [edit])
> This doesn't refresh controls properly.

A little more info on that one. Testcase? Is the problem with label @ref?

Comment 23

[Allan Beaufour]

Attachment: smaug's bug?

Ok, I'll create one myself then. Attributes are not showing up as dependencies.

Comment 24

[Allan Beaufour]

The following is not updated properly:

1) <label>. I have that fixed locally.
2) binding to attributes
3) <output>s using @value

2 and 3 could be related, and hiding in the XPath analyzer. I'll dig into that
tomorrow.

Comment 25

[Allan Beaufour]

Attachment: smaug's bug (xhtml)

Oops, forgot to run my test through xslt, before uploading.

Comment 26

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

(In reply to comment #22)
> 
> A little more info on that one. Testcase? Is the problem with label @ref?

Sorry, I lost my network connection just when I was uploading the testcase.
But you found the bug anyway :)

Comment 27

[Allan Beaufour]

Attachment: MDG patch, updating all controls

Ok, the comments I'm getting are mostly about controls not being updated.
Here's a patch with a quick hack, that always updates all controls (@bug at
nsXFormsModelElement.cpp:545). It also fixes the label (issue 1 above).

Basically this should get your demo rolling, with all the new stuff, without
being bugged by the dependency bugs.

Comment 28

[aaronr]

There is still a couple of problems.  In nsXFormsModelElement.cpp ::Revalidate()
it is possible that curChanged will be nsnull.  With the latest patch, rebind
will always be PR_TRUE, so DispatchEvents can be called with curChange = nsnull.
This will cause mMDG.GetNodeState() in DispatchEvents to return 0, and there
isn't a test for the return.  So the instruction ns->ShouldDispatchValid() will
trap.

Also, if you select an item in a SelectElement, controls won't be updated. 
There is no recalculate, refresh, etc. generated.  

I will attach testcases.

Comment 29

[aaronr]

Attachment: testcase for trap

testcase for the trap.	Crashes with this latest patch and branch from 01/12.

Comment 30

[aaronr]

Attachment: testcase for select value update

testcase that shows that selecting a value in a select list box won't update
other controls

Comment 31

[Allan Beaufour]

(In reply to comment #28)
> There is still a couple of problems.  In nsXFormsModelElement.cpp ::Revalidate()
> it is possible that curChanged will be nsnull.  With the latest patch, rebind
> will always be PR_TRUE, so DispatchEvents can be called with curChange = nsnull.
> This will cause mMDG.GetNodeState() in DispatchEvents to return 0, and there
> isn't a test for the return.  So the instruction ns->ShouldDispatchValid() will
> trap.

Correct! But who was the joker that gave curChanged to DispatchEvents in the
first place. Cannot have been me. Must have been an evil spirit. ;-) The correct
is of course the updated boundNode, I'll fix that and include a null-check for
good measures.

Comment 32

[Allan Beaufour]

(In reply to comment #28)
> Also, if you select an item in a SelectElement, controls won't be updated. 
> There is no recalculate, refresh, etc. generated.  

I'll fix that, but select has a bug with regards to multiple selected values
(see bug 278207), so the event dispatching will probably have to be moved. I'll
note that in the appropriate place in the patch.

Comment 33

[Allan Beaufour]

Attachment: MDG patch, revision 2^26

This one fixes Aaron's comments. There's still something fishy with the events
being sent though... so the "pseudo-attributes" are not guaranteed to work.

Comment 34

[Allan Beaufour]

Attachment: MDG patch, revision 2^27

found the fish, it was swimming in the MDG engine. I had actually killed it
before (lovely!), but it must have slipped my fingers during updates or the
branch-switch. So the "Small test form" in attachment 166843 [details] works again!

So I proudly present the newest edition of the patch from hell. It still
* has non IDL-interfaces in IDL-interfaces
* does not use COMArray
* has a bug in the dependency analyzer (but the hack makes the stuff happen
anyways).

But could we create new bugs for those?

Comment 35

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

Comment on attachment 171157 [details] [diff] [review]
MDG patch, revision 2^27

(In reply to comment #34)
> 
> But could we create new bugs for those?

I think we could.


+    /// @bug This should be set to PR_FALSE! (XXX)
+    ///      Setting it to PR_TRUE rebinds all controls all the time

Remember to file a bug on this.


+nsXFormsMDGEngine::ComputeMIP(eFlag_t		aStateFlag,
+			       eFlag_t		aDispatchFlag,
+			       nsXFormsMDGNode *aNode,
+			       PRBool	       &aDidChange)
 {
-  NS_ENSURE_ARG_POINTER(aNode);
+  NS_ENSURE_ARG(aNode);
+
+  if (!aNode->mExpression)
+    return NS_OK;
Shouldn't you set aDidChange to PR_FALSE before return.

Comment 36

[Brian Ryner (not reading)]

Comment on attachment 171157 [details] [diff] [review]
MDG patch, revision 2^27

>+PRBool
>+nsXFormsNodeState::IsValid() const
>+{
>+  // constraints determine the validity of a node
>+  ///
>+  /// @todo needs Schema support (XXX)
>+  return Test(eFlag_CONSTRAINT);
>+}

It would probably be worthwhile to inline these methods that just call Test().
>+#ifdef DEBUG_XF_NODESTATE
>+#include <cstdio>

Is this for printf?  Just use <stdio.h>.  Same applies if you did this in other
files.

Looks ok otherwise, we're probably not going to find any additinoal problems
here without getting it checked in.  Nice work.

Comment 37

[Allan Beaufour]

(In reply to comment #35)
> (From update of attachment 171157 [details] [diff] [review] [edit])
> (In reply to comment #34)
> +nsXFormsMDGEngine::ComputeMIP(eFlag_t		aStateFlag,
> +			       eFlag_t		aDispatchFlag,
> +			       nsXFormsMDGNode *aNode,
> +			       PRBool	       &aDidChange)
>  {
> -  NS_ENSURE_ARG_POINTER(aNode);
> +  NS_ENSURE_ARG(aNode);
> +
> +  if (!aNode->mExpression)
> +    return NS_OK;
> Shouldn't you set aDidChange to PR_FALSE before return.

Ouch. Of course. Well spotted.

Comment 38

[Allan Beaufour]

Attachment: MDG final patch

Fixed comments from smaug and bryner

Comment 39

[Allan Beaufour]

Commited to branch

Comment 40

[Allan Beaufour]

Checked in to trunk via bug 278896.

Comment 41

[Allan Beaufour]

*** Bug 277475 has been marked as a duplicate of this bug. ***