Closed
Bug 307093
Opened 19 years ago
Closed 19 years ago
enumerating of model instances
Categories
(Core Graveyard :: XForms, enhancement)
Core Graveyard
XForms
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: surkov, Assigned: allan)
Details
(Keywords: fixed1.8.0.2, fixed1.8.1)
Attachments
(4 files, 4 obsolete files)
2.51 KB,
application/xhtml+xml
|
Details | |
2.09 KB,
application/xhtml+xml
|
Details | |
24.73 KB,
patch
|
Details | Diff | Splinter Review | |
1.27 KB,
patch
|
doronr
:
review+
allan
:
review+
|
Details | Diff | Splinter Review |
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 Since model can have a number of instances then nsIXFormsModelElement inteface should be able to enumerate instances. In instance, it can be achived by adding method like getInstanceDocumentList() or using nsIEnumerator interface. Reproducible: Always
Changes to nsIXFormsModelElement interface need to come from the W3C. We do have a method in the nsIModelElementPrivate interface to get a list of instances contained by the model in a nsCOMArray. Please check it out and let us know if it meets your needs.
Reporter | ||
Comment 2•19 years ago
|
||
I guess it is what I need.
Status: UNCONFIRMED → RESOLVED
Closed: 19 years ago
Resolution: --- → WORKSFORME
I'm reopening this bug for a couple of reasons: 1) Gotta keep track and somehow note interfaces that users are going to use so that we can 'freeze' them at some point or at least make sure they are appropriately marked so that they don't suddenly go away. 2) Allan, this might be something to bring up with the working group as a 1.1 enhancement to the model interface. While using DOM a user can get all the instance elements that were authored, there is no way to currently get ALL of the instance elements since lazy authored instance elements will probably not appear in the DOM for all implementations (they wont' for us). After these items are addressed, we can close this
Status: RESOLVED → UNCONFIRMED
Resolution: WORKSFORME → ---
Assignee | ||
Comment 4•19 years ago
|
||
(In reply to comment #3) > 2) Allan, this might be something to bring up with the working group as a 1.1 > enhancement to the model interface. While using DOM a user can get all the > instance elements that were authored, there is no way to currently get ALL of > the instance elements since lazy authored instance elements will probably not > appear in the DOM for all implementations (they wont' for us). I brought it up at the f2f last week, and got the action item for it. So I have to define how it should work.
Assignee: aaronr → allan
Status: UNCONFIRMED → NEW
Ever confirmed: true
Reporter | ||
Comment 6•19 years ago
|
||
(In replay to comment 1 of bug 309294) > If you have any suggestions to > exactly how the functions should look like / behave, please write it on > that bug. the first approach interface nsIXFormsModelElement: dom::Element { //specs methods dom::Document getFirstInstanceDocument(); dom::Document getNextInstanceDocument(); } the second approach interface nsIXFormsModelElement: dom::Element { //specs methods dom::NodeList getInstanceDocumentsList(); } NodeList can be replaced by some DocumentList interface (something like) interface DocumentList: dom::NodeList{ dom::Document item(in unsigned long index); }
Reporter | ||
Comment 7•19 years ago
|
||
I guess nsIXFormsModelElement should give a way to control instance data. I want have interface to modify model item properties like it do nsIXFormsDelegate interface. In instance: interface nsIXFormsModelElement: dom::Element { //specs methods nsIXFormsModelItem getModelItem(in string xpath); } interface nsIXFormsModelItem: dom::Node (?){ attribute string type; attribute string (boolean?) readonly; //others model item properties attribute string value; readonly attribute boolean valid; } Now if I want to change instance data value then I should it do by using some XForms control (by nsIXFormsDelegate interface). If I want to change some model item property then I should have xforms:bind element and change it attributes and then update model. I belive it's not right. What do you think?
Assignee | ||
Comment 8•19 years ago
|
||
(In reply to comment #7) > I guess nsIXFormsModelElement should give a way to control instance data. I want > have interface to modify model item properties like it do nsIXFormsDelegate > interface. In instance: > > interface nsIXFormsModelElement: dom::Element { > //specs methods > nsIXFormsModelItem getModelItem(in string xpath); > > } > > interface nsIXFormsModelItem: dom::Node (?){ > attribute string type; > attribute string (boolean?) readonly; > //others model item properties > > attribute string value; > readonly attribute boolean valid; > } > > Now if I want to change instance data value then I should it do by using some > XForms control (by nsIXFormsDelegate interface). If I want to change some model > item property then I should have xforms:bind element and change it attributes > and then update model. I belive it's not right. What do you think? What you want is the interface described/revised in bug 306764, but just directly on the instance data node instead of the UI control, right?
Assignee | ||
Comment 9•19 years ago
|
||
(In reply to comment #6) > (In replay to comment 1 of bug 309294) > > > If you have any suggestions to > > exactly how the functions should look like / behave, please write it on > > that bug. > > the first approach > interface nsIXFormsModelElement: dom::Element { > //specs methods > dom::Document getFirstInstanceDocument(); > dom::Document getNextInstanceDocument(); > } I'm not too fond of that approach, the DOM interfaces I've seen so far have stateless approaches. > the second approach > interface nsIXFormsModelElement: dom::Element { > //specs methods > dom::NodeList getInstanceDocumentsList(); > } That seems like the natural way to do it, but I guess we still need to have a way to figure out the id of the instance document you have actually retrieved? So we should also have an nsIXFormsInstanceDocument.getInstanceId() function, or just go through the .getInstanceElement() function from bug 305763, and get the id from the returned element. What do you say?
Reporter | ||
Comment 10•19 years ago
|
||
(In reply to comment #8) > What you want is the interface described/revised in bug 306764, but just > directly on the instance data node instead of the UI control, right? Right. Thease properties are properties of model. I guess should be possibility to change them from model. I think sometimes I may not know whay exactly controls are attached to specified data. It means if I want to change datas then I should it can do by model.
Reporter | ||
Comment 11•19 years ago
|
||
(In reply to comment #9) > That seems like the natural way to do it, but I guess we still need to have a > way to figure out the id of the instance document you have actually retrieved? > So we should also have an nsIXFormsInstanceDocument.getInstanceId() function, or > just go through the .getInstanceElement() function from bug 305763, and get the > id from the returned element. What do you say? The first approach. If you have in view thease methods should returns some nsIXFormsInstanceDocument interface then I agree with you. Something like this: interface nsIXFormsModelElement: dom::Element { //specs methods nsIXFormsInstanceDocument getFirstInstanceDocument(); nsIXFormsInstanceDocument getNextInstanceDocument(); } interface nsIXFormsInstanceDocument: dom::Document { string getInstanceId(); } What will be looked the second approach?
Reporter | ||
Comment 12•19 years ago
|
||
(In reply to comment #8) > What you want is the interface described/revised in bug 306764, but just > directly on the instance data node instead of the UI control, right? Yes, like nsIXFormsAccessors but wider. nsIXFormsAccessors provides model properties as readonly. I guess resembling interface for model should allow to set properties too.
Assignee | ||
Comment 13•19 years ago
|
||
(In reply to comment #12) > (In reply to comment #8) > > What you want is the interface described/revised in bug 306764, but just > > directly on the instance data node instead of the UI control, right? > > Yes, like nsIXFormsAccessors but wider. nsIXFormsAccessors provides model > properties as readonly. I guess resembling interface for model should allow to > set properties too. Model Item Properties are controlled by the model, so they can only be read only.
Assignee | ||
Comment 14•19 years ago
|
||
(In reply to comment #11) > interface nsIXFormsModelElement: dom::Element { > //specs methods > nsIXFormsInstanceDocument getFirstInstanceDocument(); > nsIXFormsInstanceDocument getNextInstanceDocument(); > } I do not like this state-full approach, but yes, the instance document should implement functionality. [scriptable] interface nsIXFormsModelElement : nsISupports { nsIDOMNodeList getInstanceDocuments(); nsIDOMDocument getInstanceDocument(in DOMString instanceID); // raises(DOMException) void rebuild(); void recalculate(); void revalidate(); void refresh(); }; And instance documents should implement: [scriptable] interface nsIXFormsInstanceDocument : nsISupports { readonly attribute DOMString instanceId; } That would do it I believe. I do not know if we can easily implement that attribute on the document though...
OS: Windows 2000 → All
Hardware: PC → All
Reporter | ||
Comment 15•19 years ago
|
||
(In reply to comment #13) > (In reply to comment #12) > > (In reply to comment #8) > > > What you want is the interface described/revised in bug 306764, but just > > > directly on the instance data node instead of the UI control, right? > > > > Yes, like nsIXFormsAccessors but wider. nsIXFormsAccessors provides model > > properties as readonly. I guess resembling interface for model should allow to > > set properties too. > > Model Item Properties are controlled by the model, so they can only be read only. But I propose interface for model element. Now I should change correspond attribute for xforms:bind and then update model. I guess such interface will be useful.
Assignee | ||
Comment 16•19 years ago
|
||
(In reply to comment #15) > (In reply to comment #13) > > (In reply to comment #12) > > > Yes, like nsIXFormsAccessors but wider. nsIXFormsAccessors provides model > > > properties as readonly. I guess resembling interface for model should allow to > > > set properties too. > > > > Model Item Properties are controlled by the model, so they can only be read only. > > But I propose interface for model element. Now I should change correspond > attribute for xforms:bind and then update model. I guess such interface will be > useful. IMHO: No way. binds control the MIPs, that's how XForms work.
Reporter | ||
Comment 17•19 years ago
|
||
(In reply to comment #16) > IMHO: No way. binds control the MIPs, that's how XForms work. I don't see a contradiction. Please take a look at comment #7. I propose nsIXFormsModelItem interface like a bind element. Now I don't see a simple way to change values of MIP. Such interface can. What do you see ugly in that?
Assignee | ||
Comment 18•19 years ago
|
||
(In reply to comment #17) > (In reply to comment #16) > > > IMHO: No way. binds control the MIPs, that's how XForms work. > > I don't see a contradiction. Please take a look at comment #7. I propose > nsIXFormsModelItem interface like a bind element. Now I don't see a simple way > to change values of MIP. Such interface can. What do you see ugly in that? I think we are talking about two different things, or you have some concepts mixed up. <bind id="bindid" nodeset="x" readonly="true()"/>, sets the "readonly" Model Item Property for 1 or more instance data nodes, so that those instance data nodes will (in theory) have .isReadonly == true. So what's wrong with document.getElementById("bindid").setAttribute("readonly", "false()")?
Reporter | ||
Comment 19•19 years ago
|
||
(In reply to comment #18) > I think we are talking about two different things, or you have some concepts > mixed up. > > <bind id="bindid" nodeset="x" readonly="true()"/>, sets the "readonly" Model > Item Property for 1 or more instance data nodes, so that those instance data > nodes will (in theory) have .isReadonly == true. > > So what's wrong with document.getElementById("bindid").setAttribute("readonly", > "false()")? Yes. What's wrong with dynamicaly changing of MIPs?
Assignee | ||
Comment 20•19 years ago
|
||
(In reply to comment #19) > (In reply to comment #18) > > > I think we are talking about two different things, or you have some concepts > > mixed up. > > > > <bind id="bindid" nodeset="x" readonly="true()"/>, sets the "readonly" Model > > Item Property for 1 or more instance data nodes, so that those instance data > > nodes will (in theory) have .isReadonly == true. > > > > So what's wrong with document.getElementById("bindid").setAttribute("readonly", > > "false()")? > > Yes. What's wrong with dynamicaly changing of MIPs? Nothing, if you do the above and do it through the binds. The model bindings are applied to the data. You cannot set properties directly on data.
Reporter | ||
Comment 21•19 years ago
|
||
(In reply to comment #20) > Nothing, if you do the above and do it through the binds. The model bindings are > applied to the data. You cannot set properties directly on data. So what about interface for MIPs setting through the binds (something like comment #7)?
Assignee | ||
Comment 22•19 years ago
|
||
(In reply to comment #21) > (In reply to comment #20) > > Nothing, if you do the above and do it through the binds. The model bindings are > > applied to the data. You cannot set properties directly on data. > > So what about interface for MIPs setting through the binds (something like > comment #7)? It's not needed. You can set the properties by changing the attributes through the DOM.
Reporter | ||
Comment 23•19 years ago
|
||
(In reply to comment #22) > > It's not needed. You can set the properties by changing the attributes through > the DOM. Will be controls state are changed when I'll change attribute of bind element? There is have a sense too when bind element is absent for some instance data. I guess it's more comfortable to set some MIPs values as bolean values (not as xpath string).
Assignee | ||
Comment 24•19 years ago
|
||
(In reply to comment #23) > (In reply to comment #22) > > > > It's not needed. You can set the properties by changing the attributes through > > the DOM. > > Will be controls state are changed when I'll change attribute of bind element? Nope. Binds are static, you need to do a rebuild, etc. > There is have a sense too when bind element is absent for some instance data. ? > I guess it's more comfortable to set some MIPs values as bolean values (not as > xpath string). Possibly, but it's not XForms.
Reporter | ||
Comment 25•19 years ago
|
||
(In reply to comment #24) Interface is assigned for straight working with data. I can set/get MIPs don't using bind element obviously. I'm able to set/get data value don't using xforms controls. Advantages during work with MIPs: 1) I shouldn't rebuild model. > > There is have a sense too when bind element is absent for some instance data. > ? 2) That means I shouldn't have bind element to set some property > > I guess it's more comfortable to set some MIPs values as bolean values (not as > > xpath string). > Possibly, but it's not XForms. nsIXFormsDelegate does it.
Assignee | ||
Comment 26•19 years ago
|
||
(In reply to comment #25) > (In reply to comment #24) > > Interface is assigned for straight working with data. I can set/get MIPs don't > using bind element obviously. I'm able to set/get data value don't using xforms > controls. Advantages during work with MIPs: > 1) I shouldn't rebuild model. > > > > There is have a sense too when bind element is absent for some instance data. > > ? > 2) That means I shouldn't have bind element to set some property That's possible, but unfortunately that's not XForms. > > > I guess it's more comfortable to set some MIPs values as bolean values (not as > > > xpath string). > > > Possibly, but it's not XForms. > > nsIXFormsDelegate does it. Nope. Delegate returns the state for a MIP, which naturally is boolean. It cannot set it (it's readonly attributes).
Assignee | ||
Comment 27•19 years ago
|
||
This patch creates an nsXFormsInstanceDocuments class that maintains the list of instances for the model (instead of the nsCOMArray in the model) and implemen ts nsIDOMNodeList, so it can be returned by the new getInstanceDocuments(). I modified existing users of model->GetInstanceList to use this instead. I'm not happy about the mInstanceDocuments.AddRef()/Release() in nsXFormsModelElement, but using a nsCOMPtr keeps giving me "ambiguous nsISupport" errors.
Attachment #199790 -
Flags: review?(aaronr)
Assignee | ||
Comment 28•19 years ago
|
||
Comment 29•19 years ago
|
||
Comment on attachment 199790 [details] [diff] [review] Patch >diff -X patch-excludes -uprN -U8 xforms/nsXFormsModelElement.h xforms.instances/nsXFormsModelElement.h >--- xforms/nsXFormsModelElement.h 2005-08-05 15:26:39.000000000 +0200 >+++ xforms.instances/nsXFormsModelElement.h 2005-10-12 15:58:36.000000000 +0200 >@@ -203,20 +251,20 @@ private: > // This flag indicates whether or not the document fired DOMContentLoaded > PRBool mDocumentLoaded; > > // This flag indicates whether a xforms-rebuild has been called, but no > // xforms-revalidate yet > PRBool mNeedsRefresh; > > /** >- * List of instance elements contained by this model, including lazy-authored >+ * All instance documents contained by this model, including lazy-authored > * instance elements. > */ nit: should probably be, "...including lazy-authored instance documents." You should probably add a testcase to show that this works with lazy instance, too. Patch looks good, though. r=me
Attachment #199790 -
Flags: review?(aaronr) → review+
Assignee | ||
Comment 30•19 years ago
|
||
Includes illegal index testing which triggers a bug in the patch.
Assignee | ||
Comment 31•19 years ago
|
||
This fixes your nits, and uses SafeObjectAt() instead of ObjectAt() in Item()
Attachment #199790 -
Attachment is obsolete: true
Attachment #200068 -
Flags: review?(aaronr)
Attachment #200068 -
Flags: review?(aaronr) → review+
Assignee | ||
Updated•19 years ago
|
Attachment #200068 -
Flags: review?(smaug)
Comment 32•19 years ago
|
||
Comment on attachment 200068 [details] [diff] [review] Nit and Item() fix >diff -X patch-excludes -uprN -U8 xforms/nsIXFormsModelElement.idl xforms.instances/nsIXFormsModelElement.idl >--- xforms/nsIXFormsModelElement.idl 2005-08-05 15:26:39.000000000 +0200 >+++ xforms.instances/nsIXFormsModelElement.idl 2005-10-11 15:16:10.000000000 +0200 >@@ -34,30 +34,32 @@ > * the provisions above, a recipient may use your version of this file under > * the terms of any one of the MPL, the GPL or the LGPL. > * > * ***** END LICENSE BLOCK ***** */ > > #include "domstubs.idl" > > interface nsIDOMDocument; >+interface nsIDOMNodeList; > interface nsIDOMDOMException; > > /** > * Defines scriptable methods for manipulating instance data > * and updating computed and displayed values. > * > * For more information on this interface please see > * http://www.w3.org/TR/xforms/ > * > * @status FROZEN > */ >-[scriptable, uuid(e891d132-46f8-464c-b93b-1fb883fa03da)] >+[scriptable, uuid(85fd60c7-1db7-40c0-ae8d-f723fdd1eea8)] > interface nsIXFormsModelElement : nsISupports > { >+ nsIDOMNodeList getInstanceDocuments(); > nsIDOMDocument getInstanceDocument(in DOMString instanceID); > // raises(DOMException) > > void rebuild(); > void recalculate(); > void revalidate(); > void refresh(); > }; nsIXformsModelElement.idl is FROZEN, you can't change that. You should create nsIXFormsModelElement2 (or what to call it) interface which has nsIDOMNodeList getInstanceDocuments() method > /** >- * List of instance elements contained by this model, including lazy-authored >- * instance elements. >+ * All instance documents contained by this model, including lazy-authored >+ * instance documents. > */ >- nsCOMArray<nsIInstanceElementPrivate> mInstanceList; >+ nsXFormsModelInstanceDocuments mInstanceDocuments; I'd prefer nsRefPtr<nsXFormsModelInstanceDocuments> mInstanceDocuments; Then create the instance document list when it is first time needed. nsRefPtr addrefs automatically. > > /** Indicates whether the model's instance was built by lazy authoring */ > PRBool mLazyModel; > }; > > /** > * nsPostRefresh is needed by the UI Controls, which are implemented in > * XBL and used inside \<repeat\>. It is needed to refresh the controls, >diff -X patch-excludes -uprN -U8 xforms/nsXFormsUtilityService.cpp xforms.instances/nsXFormsUtilityService.cpp >--- xforms/nsXFormsUtilityService.cpp 2005-08-05 15:26:39.000000000 +0200 >+++ xforms.instances/nsXFormsUtilityService.cpp 2005-10-12 13:52:59.000000000 +0200 >@@ -108,70 +108,43 @@ nsXFormsUtilityService::GetModelFromNode > * xforms:repeat. > */ > NS_IMETHODIMP > nsXFormsUtilityService::IsNodeAssocWithModel( nsIDOMNode *aNode, > nsIDOMNode *aModel, > PRBool *aModelAssocWithNode) > { > >- nsCOMPtr<nsIDOMElement> element = do_QueryInterface(aNode); >+ nsCOMPtr<nsIDOMNode> modelNode; > > nsAutoString namespaceURI; > aNode->GetNamespaceURI(namespaceURI); >- > // If the node is in the XForms namespace and XTF based, then it should > // be able to be handled by GetModel. Otherwise it is probably an instance > // node in a instance document. > if (namespaceURI.EqualsLiteral(NS_NAMESPACE_XFORMS)) { >+ nsCOMPtr<nsIDOMElement> element = do_QueryInterface(aNode); > nsCOMPtr<nsIModelElementPrivate> modelPriv = nsXFormsUtils::GetModel(element); >- nsCOMPtr<nsIDOMNode> modelNode = do_QueryInterface(modelPriv); >- >- if (modelNode && (modelNode == aModel)) { >- *aModelAssocWithNode = PR_TRUE; >- } else { >- *aModelAssocWithNode = PR_FALSE; >- } >+ modelNode = do_QueryInterface(modelPriv); > } else { > // We are assuming that if the node coming in isn't a proper XForms element, > // then it is an instance element in an instance doc. Now we just have > // to determine if the given model contains this instance document. >- nsCOMPtr<nsIDOMDocument> document; >- aNode->GetOwnerDocument(getter_AddRefs(document)); >- *aModelAssocWithNode = PR_FALSE; >- >- // Guess that we'd better make sure that it is a model >- nsCOMPtr<nsIXFormsModelElement> modelEle = do_QueryInterface(aModel); >- if (modelEle) { >- // OK, we know that this is a model element. So now we have to go >- // instance element by instance element and find the associated >- // document. If it is equal to the document that contains aNode, >- // then aNode is associated with this aModel element and we can return >- // true. >- nsCOMPtr<nsIModelElementPrivate>model = do_QueryInterface(modelEle); >- nsCOMPtr<nsIInstanceElementPrivate> instElement; >- nsCOMPtr<nsIDOMDocument> instDocument; >- >- nsCOMArray<nsIInstanceElementPrivate> *instList = nsnull; >- model->GetInstanceList(&instList); >- NS_ENSURE_TRUE(instList, NS_ERROR_FAILURE); >- >- for (int i = 0; i < instList->Count(); ++i) { >- instElement = instList->ObjectAt(i); >- >- instElement->GetDocument(getter_AddRefs(instDocument)); >- if (instDocument) { >- if (instDocument == document) { >- *aModelAssocWithNode = PR_TRUE; >- break; >- } >- } >- } >+ nsCOMPtr<nsIDOMNode> instNode; >+ nsresult rv = >+ nsXFormsUtils::GetInstanceNodeForData(aNode, getter_AddRefs(instNode)); >+ if (NS_SUCCEEDED(rv) && instNode) { >+ instNode->GetParentNode(getter_AddRefs(modelNode)); > } >+ } > >+ if (modelNode && (modelNode == aModel)) { >+ *aModelAssocWithNode = PR_TRUE; >+ } else { >+ *aModelAssocWithNode = PR_FALSE; > } > > return NS_OK; > } > > NS_IMETHODIMP > nsXFormsUtilityService::GetInstanceDocumentRoot(const nsAString& aID, > nsIDOMNode *aModelNode, >diff -X patch-excludes -uprN -U8 xforms/nsXFormsUtils.cpp xforms.instances/nsXFormsUtils.cpp >--- xforms/nsXFormsUtils.cpp 2005-10-19 09:26:19.000000000 +0200 >+++ xforms.instances/nsXFormsUtils.cpp 2005-10-19 13:18:44.000000000 +0200 >@@ -560,20 +560,20 @@ nsXFormsUtils::EvaluateNodeBinding(nsIDO > if (colon) { > nsCOMPtr<nsIDOM3Node> dom3node = do_QueryInterface(aElement); > rv = dom3node->LookupNamespaceURI(Substring(expr.get(), colon), > namespaceURI); > NS_ENSURE_SUCCESS(rv, rv); > } > > if (NS_SUCCEEDED(rv)) { >- nsCOMArray<nsIInstanceElementPrivate> *instList = nsnull; >- (*aModel)->GetInstanceList(&instList); >- nsCOMPtr<nsIInstanceElementPrivate> instance = >- instList->ObjectAt(0); >+ nsCOMPtr<nsIInstanceElementPrivate> instance; >+ rv = (*aModel)->FindInstanceElement(EmptyString(), >+ getter_AddRefs(instance)); >+ NS_ENSURE_SUCCESS(rv, rv); > nsCOMPtr<nsIDOMDocument> domdoc; > instance->GetDocument(getter_AddRefs(domdoc)); > nsCOMPtr<nsIDOMElement> instanceDataEle; > nsCOMPtr<nsIDOMNode> childReturn; > rv = domdoc->CreateElementNS(namespaceURI, expr, > getter_AddRefs(instanceDataEle)); > NS_ENSURE_SUCCESS(rv, rv); > nsCOMPtr<nsIDOMElement> instanceRoot;
Attachment #200068 -
Flags: review?(smaug) → review-
Assignee | ||
Comment 34•19 years ago
|
||
(In reply to comment #32) > (From update of attachment 200068 [details] [diff] [review] [edit]) > nsIXformsModelElement.idl is FROZEN, you can't change that. > You should create nsIXFormsModelElement2 (or what to call it) interface > which has nsIDOMNodeList getInstanceDocuments() method You are absolutely right! Done. > >+ nsXFormsModelInstanceDocuments mInstanceDocuments; > > I'd prefer nsRefPtr<nsXFormsModelInstanceDocuments> mInstanceDocuments; > Then create the instance document list when it is first time needed. > nsRefPtr addrefs automatically. That was the solution. Done.
Attachment #200068 -
Attachment is obsolete: true
Attachment #200337 -
Flags: review?(smaug)
Comment 35•19 years ago
|
||
Comment on attachment 200337 [details] [diff] [review] With Olli's comments > > nsXFormsModelElement::nsXFormsModelElement() > : mElement(nsnull), > mSchemaCount(0), > mSchemaTotal(0), > mPendingInstanceCount(0), > mDocumentLoaded(PR_FALSE), > mNeedsRefresh(PR_FALSE), >- mInstanceList(16), > mLazyModel(PR_FALSE) > { >+ mInstanceDocuments = new nsXFormsModelInstanceDocuments(); >+ NS_ASSERTION(mInstanceDocuments, "could not create mInstanceDocuments?!"); > } Hmm, using |new| in a constructor. You can't really handle OOM this way. Maybe create mInstanceDocuments in OnCreate(). Because of OOM you should always check whether mInstanceDocuments is null. With OOM handling, r=me
Attachment #200337 -
Flags: review?(smaug) → review+
Assignee | ||
Comment 36•19 years ago
|
||
(In reply to comment #35) > (From update of attachment 200337 [details] [diff] [review] [edit]) > > > > nsXFormsModelElement::nsXFormsModelElement() > > : mElement(nsnull), > > mSchemaCount(0), > > mSchemaTotal(0), > > mPendingInstanceCount(0), > > mDocumentLoaded(PR_FALSE), > > mNeedsRefresh(PR_FALSE), > >- mInstanceList(16), > > mLazyModel(PR_FALSE) > > { > >+ mInstanceDocuments = new nsXFormsModelInstanceDocuments(); > >+ NS_ASSERTION(mInstanceDocuments, "could not create mInstanceDocuments?!"); > > } > > > Hmm, using |new| in a constructor. > You can't really handle OOM this way. > Maybe create mInstanceDocuments in OnCreate(). > Because of OOM you should always check whether > mInstanceDocuments is null. > > With OOM handling, r=me I should listen to myself... I wrote it, and thought ... "hmmm, is this really good?". Sorry. Fixed
Attachment #200337 -
Attachment is obsolete: true
Attachment #200603 -
Flags: review?(doronr)
Comment 37•19 years ago
|
||
Comment on attachment 200603 [details] [diff] [review] Patch w/proper OOM handling Ugh, nsIXFormsModelElement2. Perhaps nsIXFormsNSModelElement :) (NS being non standard), but 2 is fine as well.
Attachment #200603 -
Flags: review?(doronr) → review+
Assignee | ||
Comment 38•19 years ago
|
||
Checked in to trunk
Status: NEW → ASSIGNED
Whiteboard: xf-to-branch
Assignee | ||
Comment 39•19 years ago
|
||
This is the version checked in.
Attachment #200603 -
Attachment is obsolete: true
Comment 40•19 years ago
|
||
The patch had wrong QI implementation, and (I think) because of that I get ###!!! ASSERTION: QueryInterface needed: 'query_result.get() == mRawPtr', file ../../../dist/include/xpcom/nsCOMPtr.h, line 594 Break: at file ../../../dist/include/xpcom/nsCOMPtr.h, line 594 when testing https://bugzilla.mozilla.org/attachment.cgi?id=200906
Attachment #201029 -
Flags: review?(allan)
Updated•19 years ago
|
Attachment #201029 -
Flags: review?(doronr)
Updated•19 years ago
|
Attachment #201029 -
Flags: review?(allan) → review+
Assignee | ||
Updated•19 years ago
|
Attachment #201029 -
Flags: review?(doronr) → review+
Comment 41•19 years ago
|
||
(In reply to comment #40) > Created an attachment (id=201029) [edit] > Fix for the QI > > The patch had wrong QI implementation, and (I think) because of that I get > ###!!! ASSERTION: QueryInterface needed: 'query_result.get() == mRawPtr', file > ../../../dist/include/xpcom/nsCOMPtr.h, line 594 > Break: at file ../../../dist/include/xpcom/nsCOMPtr.h, line 594 > when testing https://bugzilla.mozilla.org/attachment.cgi?id=200906 > Checked in
Comment 42•19 years ago
|
||
checked into MOZILLA_1_8_BRANCH via bug 323691. Leaving open for now until it gets into 1.8.0
Assignee | ||
Updated•19 years ago
|
Whiteboard: xf-to-branch
Assignee | ||
Updated•19 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 19 years ago → 19 years ago
Keywords: fixed1.8.0.2
Resolution: --- → FIXED
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•