Last Comment Bug 308106 - controls not bound to external instance data in xul
: controls not bound to external instance data in xul
: fixed1.8.0.2, fixed1.8.1
Product: Core Graveyard
Classification: Graveyard
Component: XForms (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Olli Pettay [:smaug]
: Stephen Pride
Depends on:
  Show dependency treegraph
Reported: 2005-09-11 23:07 PDT by alexander :surkov
Modified: 2016-07-15 14:46 PDT (History)
4 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---

xml data (215 bytes, text/xml)
2005-09-11 23:10 PDT, alexander :surkov
no flags Details
testcase (1.29 KB, application/vnd.mozilla.xul+xml)
2005-09-11 23:11 PDT, alexander :surkov
no flags Details
test case, XUL + <xforms:model/> + lazy instance (612 bytes, application/vnd.mozilla.xul+xml)
2005-10-19 11:58 PDT, Olli Pettay [:smaug]
no flags Details
Model initializes instances (17.85 KB, patch)
2005-10-19 12:19 PDT, Olli Pettay [:smaug]
allan: review+
aaronr: review+
Details | Diff | Splinter Review
patch (18.05 KB, patch)
2005-10-28 11:51 PDT, Olli Pettay [:smaug]
no flags Details | Diff | Splinter Review

Description alexander :surkov 2005-09-11 23:07:54 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20050821 Firefox/1.6a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20050821 Firefox/1.6a1

The bug like a bug 279021. When I try to use xforms in xul then I get an exception:

Error: XForms Error (16): Could not bind control to instance data
Source File: chrome://xfcalc/content/protocol2.xul
Line: 0
Source Code:
<xforms:input enabled="1" bind="firstnameemployee1" class="editField"/>

Reproducible: Always
Comment 1 alexander :surkov 2005-09-11 23:10:21 PDT
Created attachment 195712 [details]
xml data
Comment 2 alexander :surkov 2005-09-11 23:11:26 PDT
Created attachment 195713 [details]
Comment 3 Allan Beaufour 2005-10-17 06:17:08 PDT
This is funky stuff...

Bug 302915 should have fixed Begin/DoneAddingChildren() for XUL. I do not think
it did the job 100%. If I have a test form with an <xforms:instance src="..."/>
only BeginAddingChildren() are called. That means that a lazy instance is
created if a control tries to bind to anything. The lazy instance has _both_
Begin/Done() called... but only the first time. A refresh of the page only
triggers the Begin() function on the DOM instance.
Comment 4 Allan Beaufour 2005-10-17 06:54:20 PDT
(In reply to comment #3)
> The lazy instance has _both_  Begin/Done() called... but only the first time.
> A refresh of the page only triggers the Begin() function on the DOM instance.

This is the XUL cache doing tricks on us. Setting
"nglayout.debug.disable_xul_cache" to true gives consistent behaviour.
Comment 5 Olli Pettay [:smaug] 2005-10-17 09:01:57 PDT
Argh, my bug.
But it helps if you add just any content to the instance element,
for example <xforms:instance src="externalInstance.xml"> </xforms:instance>
Comment 6 Olli Pettay [:smaug] 2005-10-17 11:27:12 PDT
This should be fixed now in trunk.
Bug 312743
Comment 7 Allan Beaufour 2005-10-17 13:07:18 PDT
Confirmed, bug 312743 helped. Problem is that we missed Firefox 1.5 :(. But the
work around in comment 6 can be used.

*** This bug has been marked as a duplicate of 312743 ***
Comment 8 Olli Pettay [:smaug] 2005-10-18 23:36:28 PDT
Reopening, because we can make this work also in branch.
Atm nsXFormsInstanceElement uses begin/doneAddingChildren to initialize itself,
but it could be the nsXFormsModelElement which initilizes instances.
So adding Init() to nsIInstanceElementPrivate and
iterating child nodes in nsXFormsModelElement::DoneAddingChildren()
(that one is called)  and if the child is a nsIInstanceElementPrivate,
call the Init() (which would do the same thing
as nsXFormsInstanceElement::DoneAddingChildren() atm). 
Hmm, there is already nsXFormsInstanceElement::InitializeLazyInstance(),
that could be removed and the functionality moved to the normal Init().

There is one case this change doesn't handle. It is the empty <xforms:model/> in 
a XUL document (<xforms:model> </xforms:model> does work).
Otherwise the DoneAddingChildren bug shouldn't affect XForms.
In those UI elements which are using DoneAddingChildren,
there should anyway be some child nodes.

And it is even possible to do a small hack to fix the <xforms:model/>
problem. Model element is anyway listening DOMContentLoaded, so when that
event occurs, it can be sure that no child nodes will be added by the content 
Comment 9 Olli Pettay [:smaug] 2005-10-19 11:58:31 PDT
Created attachment 200119 [details]
test case, XUL + <xforms:model/> + lazy instance
Comment 10 Olli Pettay [:smaug] 2005-10-19 12:19:54 PDT
Created attachment 200122 [details] [diff] [review]
Model initializes instances

This merges nsXFormsInstanceElement::DoneAddingChildren and  
nsXFormsInstanceElement::InitializeLazyInstance() to
nsXFormsInstanceElement::Initialize(). Model initializes instances.
The hack which is needed for 1.8 branch (but shouldn't hurt trunk)
is the InitializeInstances() call while handling DOMContentLoaded.

Tested using trunk with and without the fix for DoneAddingChildren.
Comment 11 Allan Beaufour 2005-10-21 05:22:46 PDT
Comment on attachment 200122 [details] [diff] [review]
Model initializes instances

> -nsXFormsInstanceElement::ParentChanged(nsIDOMElement *aNewParent)
> -{
> -  if (!aNewParent || mAddingChildren || mLazy)
> -    return NS_OK;
> -
> -  // Once we are set up in the DOM, can find the model and make sure that this
> -  // instance is on the list of instance elements that model keeps
> -  nsCOMPtr<nsIModelElementPrivate> model = GetModel();
> -  if (model) {
> -    model->AddInstanceElement(this);
> -  }
> -  
> -  return NS_OK;
> -}
> -

What if the parent is changed, then? Not terribly important to me though.

> @@ -866,16 +898,21 @@ nsXFormsModelElement::HandleEvent(nsIDOM
>    if (!nsXFormsUtils::EventHandlingAllowed(aEvent, mElement))
>      return NS_OK;

>    nsAutoString type;
>    aEvent->GetType(type);
>    if (!type.EqualsLiteral("DOMContentLoaded"))
>      return NS_OK;

> +  if (!mInstancesInitialized) {
> +    // XXX This is for Bug 308106.

Please add a more descriptive comment.

I'd like a handler for ParentChanged(), but it's not important r=me
Comment 12 aaronr 2005-10-21 15:05:28 PDT
Comment on attachment 200122 [details] [diff] [review]
Model initializes instances

I'm fine with it as it is.   Nice cleanup.  But if you do go Allan's way with
keeping ::ParentChanged and having it add instance elements to the model, then
you should do ::WillParentChanged to remove instances from models if removing
the instance element from the model.  And that could cause some issues.
Comment 13 Olli Pettay [:smaug] 2005-10-28 11:51:15 PDT
Created attachment 201165 [details] [diff] [review]

I'll check in this patch. (just up-to-date with trunk and a better comment).
I didn't add the WillChangeParent/ParentChanged because that
wasn't working earlier either and it needs more changes...
and I'm not sure if we wan't that at all.
(When should we dispatch events related to model construction if instances can
be added dynamically. And what should happen to the lazy instance. etc.)
Comment 14 Olli Pettay [:smaug] 2005-10-28 11:55:59 PDT
Checked in
Comment 15 aaronr 2006-02-02 17:16:14 PST
checked into MOZILLA_1_8_BRANCH via bug 323691.  Leaving open for now until it gets into 1.8.0
Comment 16 aaronr 2006-07-07 11:44:17 PDT
verified fixed in MOZILLA_1_8_BRANCH

Note You need to log in before you can comment on or make changes to this bug.