Closed
Bug 361366
Opened 18 years ago
Closed 18 years ago
problems with xf:repeat in anonymous content
Categories
(Core Graveyard :: XForms, defect)
Core Graveyard
XForms
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: surkov, Assigned: surkov)
Details
(Keywords: fixed1.8.0.12, fixed1.8.1.4)
Attachments
(3 files, 1 obsolete file)
xforms:repeat cannot be used in anonymous content, for example: <content> <children include="label"/> <xf:repeat xbl:inherits="model, bind, nodeset"> <children/> <xf:trigger> <xf:label>R</xf:label> <xf:delete ev:event="DOMActivate" at="index()" xbl:inherits="model, bind, nodeset"/> </xf:trigger> </xf:repeat> </content> mozType:attrBased="true" placed directly on xforms:repeat will not work right here. I guess we can do some magic like DOMi do: http://lxr.mozilla.org/mozilla/source/layout/inspector/src/inDOMView.cpp#1240 Also, this will release us from mozType:attrBased stuff.
Assignee | ||
Comment 1•18 years ago
|
||
The other problem of repeat using in anonymous content is index() function. If repeat is placed inside anonymous content then how can index() function refer on that repeat. We can't use id attribute to idenfify repeat. Can we assume that index() method is referred on parent repeat if argument of index() is missed?
(In reply to comment #1) > The other problem of repeat using in anonymous content is index() function. If > repeat is placed inside anonymous content then how can index() function refer > on that repeat. We can't use id attribute to idenfify repeat. Can we assume > that index() method is referred on parent repeat if argument of index() is > missed? > Not according to spec. According to spec, index must have a parameter.
Assignee | ||
Comment 3•18 years ago
|
||
(In reply to comment #2) > (In reply to comment #1) > > The other problem of repeat using in anonymous content is index() function. If > > repeat is placed inside anonymous content then how can index() function refer > > on that repeat. We can't use id attribute to idenfify repeat. Can we assume > > that index() method is referred on parent repeat if argument of index() is > > missed? > > > > Not according to spec. According to spec, index must have a parameter. > Then, can we assume that 'anonid' attribute that is used inside anonymous content is ID attribute? In this case we can specify 'anonid' as argument of index() method.
(In reply to comment #3) > (In reply to comment #2) > > (In reply to comment #1) > > > The other problem of repeat using in anonymous content is index() function. If > > > repeat is placed inside anonymous content then how can index() function refer > > > on that repeat. We can't use id attribute to idenfify repeat. Can we assume > > > that index() method is referred on parent repeat if argument of index() is > > > missed? > > > > > > > Not according to spec. According to spec, index must have a parameter. > > > > Then, can we assume that 'anonid' attribute that is used inside anonymous > content is ID attribute? In this case we can specify 'anonid' as argument of > index() method. > I don't think that I'd have a problem with that, as long as the precedence of the search would be for regular DOM id's first and then if that isn't found, then via anonid.
Assignee | ||
Comment 5•18 years ago
|
||
(In reply to comment #4) > > Then, can we assume that 'anonid' attribute that is used inside anonymous > > content is ID attribute? In this case we can specify 'anonid' as argument of > > index() method. > > > > I don't think that I'd have a problem with that, as long as the precedence of > the search would be for regular DOM id's first and then if that isn't found, > then via anonid. > I filed bug 361501 for this.
Assignee | ||
Comment 6•18 years ago
|
||
Assignee: xforms → surkov.alexander
Status: NEW → ASSIGNED
Attachment #246274 -
Flags: review?(aaronr)
Assignee | ||
Updated•18 years ago
|
Attachment #246274 -
Flags: review?(doronr)
Assignee | ||
Comment 7•18 years ago
|
||
Updated•18 years ago
|
Attachment #246274 -
Flags: review?(doronr) → review+
Comment on attachment 246274 [details] [diff] [review] patch >Index: extensions/xforms/nsXFormsRepeatElement.cpp >=================================================================== >RCS file: /cvsroot/mozilla/extensions/xforms/nsXFormsRepeatElement.cpp,v >retrieving revision 1.38 >diff -u -8 -p -r1.38 nsXFormsRepeatElement.cpp >--- extensions/xforms/nsXFormsRepeatElement.cpp 8 Oct 2006 14:15:01 -0000 1.38 >+++ extensions/xforms/nsXFormsRepeatElement.cpp 22 Nov 2006 14:03:53 -0000 >@@ -42,23 +42,22 @@ > #include "nsISchema.h" > #include "nsIServiceManager.h" > #include "nsMemory.h" > #include "nsString.h" > #include "nsSubstring.h" > > #include "nsIDOM3EventTarget.h" > #include "nsIDOM3Node.h" >-#include "nsIDOMDOMImplementation.h" > #include "nsIDOMDocument.h" > #include "nsIDOMElement.h" >-#include "nsIDOMEventTarget.h" > #include "nsIDOMNodeList.h" > #include "nsIDOMXPathResult.h" >-#include "nsIDOMDocumentXBL.h" >+#include "nsIContent.h" >+#include "nsIBindingManager.h" > > #include "nsXFormsControlStub.h" > #include "nsIXFormsContextControl.h" > #include "nsIXFormsRepeatElement.h" > #include "nsIXFormsRepeatUIElement.h" > #include "nsIXFormsRepeatItemElement.h" > #include "nsXFormsAtoms.h" > #include "nsXFormsModelElement.h" >@@ -234,22 +233,16 @@ protected: > * Is the repeat attribute-based (ie. created by using xf:repeat-* > * attributes). > * > * @see http://www.w3.org/TR/xforms/slice9.html#ui.repeat.via.attrs > */ > PRPackedBool mIsAttributeBased; > > /** >- * A clone of the parent node (used when mIsAttributeBased), that is used by >- * InsertTemplateContent(). >- */ >- nsCOMPtr<nsIDOMNode> mParentClone; >- >- /** > * The currently selected repeat (nested repeats) > */ > nsCOMPtr<nsIXFormsRepeatElement> mCurrentRepeat; > > /** > * Array of controls using the repeat-index > */ > nsCOMArray<nsIXFormsControl> mIndexUsers; >@@ -321,25 +314,16 @@ protected: > > /** > * Insert the template content for a repeat node into the given node. > * > * @param aNode The node to insert content into. > */ > nsresult InsertTemplateContent(nsIDOMNode *aNode); > >- >- /** >- * Get the XBL binding parent for a node. >- * >- * @param aNode The node to get binding parent for >- * @param aParent The returned binding parent >- */ >- nsresult GetBindingParent(nsIDOMNode *aNode, nsIDOMNode **aParent); >- > // nsXFormsControlStub override > virtual void AfterSetAttribute(nsIAtom *aName); > > public: > NS_DECL_ISUPPORTS_INHERITED > > // nsIXTFElement overrides > NS_IMETHOD OnCreated(nsIXTFElementWrapper *aWrapper); >@@ -749,91 +733,90 @@ nsXFormsRepeatElement::Bind(PRBool *aCon > NS_ENSURE_SUCCESS(rv, rv); > > *aContextChanged = PR_TRUE; > > return NS_OK; > } > > nsresult >-nsXFormsRepeatElement::GetBindingParent(nsIDOMNode *aNode, >- nsIDOMNode **aParent) >-{ >- NS_ENSURE_ARG_POINTER(aNode); >- NS_ENSURE_ARG_POINTER(aParent); >- >- nsCOMPtr<nsIDOMDocument> domDoc; >- aNode->GetOwnerDocument(getter_AddRefs(domDoc)); >- nsCOMPtr<nsIDOMDocumentXBL> docXBL(do_QueryInterface(domDoc)); >- NS_ENSURE_STATE(docXBL); >- >- nsCOMPtr<nsIDOMElement> parent; >- docXBL->GetBindingParent(aNode, getter_AddRefs(parent)); >- NS_IF_ADDREF(*aParent = parent); >- return NS_OK; >-} >- >-nsresult > nsXFormsRepeatElement::InsertTemplateContent(nsIDOMNode *aNode) > { > NS_ENSURE_ARG_POINTER(aNode); > >+ nsCOMPtr<nsIContent> content(do_QueryInterface(mElement)); >+ NS_ENSURE_STATE(content); >+ > nsresult rv; >- nsCOMPtr<nsIDOMNode> insertionPoint; > > // If we are attribute based we need to clone our parent (the element with > // the repeat-* attributes), and insert that as the first child. > if (mIsAttributeBased) { >- NS_ENSURE_STATE(mParentClone); >- nsCOMPtr<nsIDOMNode> parentClone; >- rv = mParentClone->CloneNode(PR_FALSE, getter_AddRefs(parentClone)); >- NS_ENSURE_SUCCESS(rv, rv); >+ nsCOMPtr<nsIContent> templateContent = content->GetBindingParent(); >+ NS_ENSURE_STATE(templateContent); > >- rv = aNode->AppendChild(parentClone, getter_AddRefs(insertionPoint)); >- NS_ENSURE_SUCCESS(rv, rv); >- } else { >- insertionPoint = aNode; >- } >+ nsCOMPtr<nsIDOMNode> templateNode(do_QueryInterface(templateContent)); >+ NS_ENSURE_STATE(templateNode); > >- // Get template content >- nsCOMPtr<nsIDOMNode> child; >- if (mIsAttributeBased) { >- // If we are attribute based, we need to get our binding parent and get >- // its first child or we will be looking at the binding document. >- nsCOMPtr<nsIDOMNode> parent; >- rv = GetBindingParent(mElement, getter_AddRefs(parent)); >+ nsCOMPtr<nsIDOMNode> clonedNode; >+ rv = CloneNode(templateNode, getter_AddRefs(clonedNode)); > NS_ENSURE_SUCCESS(rv, rv); > >- rv = parent->GetFirstChild(getter_AddRefs(child)); >- NS_ENSURE_SUCCESS(rv, rv); >- } else { >- rv = mElement->GetFirstChild(getter_AddRefs(child)); >- NS_ENSURE_SUCCESS(rv, rv); >- } >+ nsCOMPtr<nsIDOMElement> clonedElm(do_QueryInterface(clonedNode)); >+ NS_ENSURE_STATE(clonedElm); > >- // Clone template >- while (child) { >- nsCOMPtr<nsIDOMNode> childClone; >- rv = CloneNode(child, getter_AddRefs(childClone)); >+ // Remove all possible repeat-* attributes >+ clonedElm->RemoveAttributeNS(NS_LITERAL_STRING(NS_NAMESPACE_XFORMS), >+ NS_LITERAL_STRING("repeat-model")); >+ clonedElm->RemoveAttributeNS(NS_LITERAL_STRING(NS_NAMESPACE_XFORMS), >+ NS_LITERAL_STRING("repeat-bind")); >+ clonedElm->RemoveAttributeNS(NS_LITERAL_STRING(NS_NAMESPACE_XFORMS), >+ NS_LITERAL_STRING("repeat-nodeset")); >+ clonedElm->RemoveAttributeNS(NS_LITERAL_STRING(NS_NAMESPACE_XFORMS), >+ NS_LITERAL_STRING("repeat-startindex")); >+ clonedElm->RemoveAttributeNS(NS_LITERAL_STRING(NS_NAMESPACE_XFORMS), >+ NS_LITERAL_STRING("repeat-number")); Why did you move this code from UnrollRows? Instead of removing the attributes once and cloning from that, we are doing the cloning first and then removing the attributes. So we'll end up removing the attributes once for every row in the repeat, right? A lot less efficient. >+ >+ nsCOMPtr<nsIDOMNode> stubNode; >+ return aNode->AppendChild(clonedNode, getter_AddRefs(stubNode)); >+ } >+ >+ nsCOMPtr<nsIDOMDocument> domDocument; >+ mElement->GetOwnerDocument(getter_AddRefs(domDocument)); >+ NS_ENSURE_STATE(domDocument); >+ >+ nsCOMPtr<nsIDocument> document(do_QueryInterface(domDocument)); >+ nsCOMPtr<nsIBindingManager> bindingMgr(document->BindingManager()); >+ NS_ENSURE_STATE(bindingMgr); >+ >+ nsCOMPtr<nsIDOMNodeList> children; >+ bindingMgr->GetContentListFor(content, getter_AddRefs(children)); You'll need comments here about why we are using this now instead of getting the children explicitly via DOM. Is this list of children guaranteed to be in document order? Removing the review request pending answers to questions. The logic of the patch seems ok other than these things.
Attachment #246274 -
Flags: review?(aaronr)
Assignee | ||
Comment 9•18 years ago
|
||
(In reply to comment #8) > Why did you move this code from UnrollRows? Instead of removing the attributes > once and cloning from that, we are doing the cloning first and then removing > the attributes. So we'll end up removing the attributes once for every row in > the repeat, right? A lot less efficient. I missed that optimization issue. Fixed. > You'll need comments here about why we are using this now instead of getting > the children explicitly via DOM. Is this list of children guaranteed to be in > document order? I guess yes. That's how DOMi works :)
Attachment #246274 -
Attachment is obsolete: true
Attachment #247394 -
Flags: review?(aaronr)
Comment 10•18 years ago
|
||
Comment on attachment 247394 [details] [diff] [review] patch2 nit: patch has window line endings. r-'ing because nested attribute-based repeats stop working with this patch. I'll attach a testcase.
Attachment #247394 -
Flags: review?(aaronr) → review-
Comment 11•18 years ago
|
||
This testcase does not show the nested repeat's outputs with the current patch.
Assignee | ||
Comment 12•18 years ago
|
||
(In reply to comment #11) > Created an attachment (id=247722) [edit] > testcase2 > > This testcase does not show the nested repeat's outputs with the current patch. > The same problem like in bug 356790. There is no XBL binding for nested repeats though binding style rules are there.
Comment 13•18 years ago
|
||
(In reply to comment #12) > (In reply to comment #11) > > Created an attachment (id=247722) [edit] > > testcase2 > > > > This testcase does not show the nested repeat's outputs with the current patch. > > > > The same problem like in bug 356790. There is no XBL binding for nested repeats > though binding style rules are there. > I tried this patch on 1.8 branch and nested repeats work, so nothing to do with this patch. Must be something on trunk.
Comment 14•18 years ago
|
||
Comment on attachment 247394 [details] [diff] [review] patch2 since the inner repeat issue I noted isn't caused by this patch, I'm r+'ing the patch. Please make sure to fix the windows line endings prior to checkin.
Attachment #247394 -
Flags: review- → review+
Assignee | ||
Comment 15•18 years ago
|
||
checked in by smaug
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch
Comment 16•17 years ago
|
||
checked into 1.8 branch on 2007-04-12 checked into 1.8.0 branch on 2007-04-16
Keywords: fixed1.8.0.12,
fixed1.8.1.4
Whiteboard: xf-to-branch
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
•