Last Comment Bug 280368 - Support attribute-based repeats (first step)
: Support attribute-based repeats (first step)
Status: RESOLVED FIXED
: fixed1.8.0.8, fixed1.8.1.1
Product: Core Graveyard
Classification: Graveyard
Component: XForms (show other bugs)
: Trunk
: All All
P5 normal with 1 vote (vote)
: ---
Assigned To: Allan Beaufour
: Stephen Pride
:
Mentors:
http://www.w3.org/TR/xforms/slice9.ht...
Depends on:
Blocks: 264329 322255 340515
  Show dependency treegraph
 
Reported: 2005-01-29 13:55 PST by Allan Beaufour
Modified: 2016-07-15 14:46 PDT (History)
8 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Testcase - div (1.09 KB, application/xhtml+xml)
2006-06-01 09:28 PDT, Allan Beaufour
no flags Details
Testcase - div, nested (1.31 KB, application/xhtml+xml)
2006-06-01 09:28 PDT, Allan Beaufour
no flags Details
Test - table (tr) (1.74 KB, application/xhtml+xml)
2006-06-01 09:29 PDT, Allan Beaufour
no flags Details
Work in progress (46.61 KB, patch)
2006-06-01 09:36 PDT, Allan Beaufour
no flags Details | Diff | Splinter Review
Testcase - div (including insert and delete) (1.67 KB, application/xhtml+xml)
2006-06-02 02:07 PDT, Allan Beaufour
no flags Details
Patch (31.58 KB, patch)
2006-06-02 02:10 PDT, Allan Beaufour
bugs: review+
Details | Diff | Splinter Review
Patch with smaug's comments fixed (31.55 KB, patch)
2006-06-02 03:25 PDT, Allan Beaufour
doronr: review+
Details | Diff | Splinter Review

Description User image Allan Beaufour 2005-01-29 13:55:15 PST
We need to support something like:
<html:table id="tab" xforms:repeat-nodeset="..." xforms:repeat-model="model2">
...
</html:table>

When implementing remember that <setindex/> and index() should still work, and
refers to the @id on the element with the attributes ("tab" in the above).
Comment 1 User image aaronr 2005-02-21 12:12:21 PST
Got a note from T.V. Raman.  We need this in to consider ourselves XForms Basic.
 Ugh!
Comment 2 User image Allan Beaufour 2005-02-21 23:59:55 PST
(In reply to comment #1)
> Got a note from T.V. Raman.  We need this in to consider ourselves XForms Basic.
>  Ugh!

Ugh white man! :) Yes, let's not forget it totally, but it's not on high on my
priority list. Let's get some of the more basic functionality working first.
Comment 3 User image alexander :surkov 2005-11-24 01:34:43 PST
If I think right then node with attributes like xforms:repeat-nodeset should have some xtf binding. But as I know xtf binding is added only by tagname. Have you any idea how it can be done?
Comment 4 User image Allan Beaufour 2005-11-24 06:10:39 PST
(In reply to comment #3)
> If I think right then node with attributes like xforms:repeat-nodeset should
> have some xtf binding. But as I know xtf binding is added only by tagname. Have
> you any idea how it can be done?

AFAIK there is no way of doing this right now. We need "XTF for attributes". Things seems to be moving towards sXBL/XBL2 though so we might need to start advocating for that there.
Comment 5 User image alexander :surkov 2005-11-24 19:28:26 PST
(In reply to comment #4)
> (In reply to comment #3)
> > If I think right then node with attributes like xforms:repeat-nodeset should
> > have some xtf binding. But as I know xtf binding is added only by tagname. Have
> > you any idea how it can be done?
> 
> AFAIK there is no way of doing this right now. We need "XTF for attributes".
> Things seems to be moving towards sXBL/XBL2 though so we might need to start
> advocating for that there.
> 

Is it too difficult to expand XTF for bindings supporting by attributes or does it contradict with XTF ideology? I guess it can takes a long time for sXBL/XBL2 awaiting. Also I think bug 306526 is in need of the such XTF possibility.

Comment 6 User image Allan Beaufour 2005-11-28 02:27:25 PST
(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > If I think right then node with attributes like xforms:repeat-nodeset should
> > > have some xtf binding. But as I know xtf binding is added only by tagname. Have
> > > you any idea how it can be done?
> > 
> > AFAIK there is no way of doing this right now. We need "XTF for attributes".
> > Things seems to be moving towards sXBL/XBL2 though so we might need to start
> > advocating for that there.
> > 
> 
> Is it too difficult to expand XTF for bindings supporting by attributes or does
> it contradict with XTF ideology? I guess it can takes a long time for sXBL/XBL2
> awaiting. Also I think bug 306526 is in need of the such XTF possibility.

I'm not sure it matters what way we go. Gecko 1.8 is closed, so we are talking long term here no matter what.
Comment 7 User image Doron Rosenberg (IBM) 2005-12-06 14:39:27 PST
theoretically, in css, we could do html|foo[xforms:attr], but that would be costly.
Comment 8 User image Allan Beaufour 2006-03-23 08:33:57 PST
This could actually work:
*[xf|repeat-nodeset] {
  -moz-binding: url("xforms.xml#repeathandler");
}

<binding id="repeathandler">
  <content>
    <xf:repeat mozType:attrbased="true" inherits="nodeset=repeat-nodeset">
      <children/>
    </xf:repeat>
  </content>
</binding>

Then in the current (C++) repeat handling of refresh/expansion:
if (attrbased) {
  template = cloneNode(this.parent, false);
  template.removeAttribute("repeat-*");
  for (c in this.children) {
    template.appendChild(c);
  }
} else {
  template = (currentcode);
}

Hacky, and slightly expensive, but it could work, and reuse the existing code.
Comment 9 User image alexander :surkov 2006-03-23 17:27:10 PST
(In reply to comment #8)
> 
> Hacky, and slightly expensive, but it could work, and reuse the existing code.
> 

Not, always.
1) if some element has binding then repeat binding overrides it. It is actual for xul.
2) probably some elements will be broken when they see xf:reapeat inside of them. F.x. I think html:select will.
Comment 10 User image Allan Beaufour 2006-03-24 00:41:49 PST
(In reply to comment #9)
> (In reply to comment #8)
> > 
> > Hacky, and slightly expensive, but it could work, and reuse the existing code.
> > 
> 
> Not, always.
> 1) if some element has binding then repeat binding overrides it. It is actual
> for xul.

No it shouldn't. Because it will be correctly bound to the cloned content inside the "unrolled" repeats.

> 2) probably some elements will be broken when they see xf:reapeat inside of
> them. F.x. I think html:select will

They would not have xf:repeat inside them. I'm not following you.
Comment 11 User image alexander :surkov 2006-03-24 00:57:22 PST
(In reply to comment #10)

> > 1) if some element has binding then repeat binding overrides it. It is actual
> > for xul.
> 
> No it shouldn't. Because it will be correctly bound to the cloned content
> inside the "unrolled" repeats.

I didn't understand. If I understand right then one element can have one binding. If it is right then binding for repeat should override default one. Am I not right?

> > 2) probably some elements will be broken when they see xf:reapeat inside of
> > them. F.x. I think html:select will
> 
> They would not have xf:repeat inside them. I'm not following you.

Probalby html:select shouldn't. But we can cover some other elements. Though in general it seems to me it's not right way to support bounded elements set.

What about xtf binding for attributes? As I can see it is one right way.
Comment 12 User image Allan Beaufour 2006-03-24 01:26:13 PST
(In reply to comment #11)
> (In reply to comment #10)
> > > 1) if some element has binding then repeat binding overrides it. It is actual
> > > for xul.
> > 
> > No it shouldn't. Because it will be correctly bound to the cloned content
> > inside the "unrolled" repeats.
> 
> I didn't understand. If I understand right then one element can have one
> binding. If it is right then binding for repeat should override default one. Am
> I not right?

You are, but look at my pseudocode. I clone the node that the repeat is bound to, without the repeat-* attributes and then used in the template. That means that:
<tr repeat-nodeset="/foo/bar"></tr>
will be bound to my repeat-thing, overriding the tr element binding, correct. But, it will be unfolded into:
<repeat nodeset="/foo/bar">
  <tr>
  </tr>
</repeat>
And thus, the original binding will be in place for the tr element in the unrolled content.

> What about xtf binding for attributes? As I can see it is one right way.

XTF is a dead end. XBL2 is where the solution should come from.
Comment 13 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2006-03-24 07:52:45 PST
(In reply to comment #12)
> XTF is a dead end. XBL2 is where the solution should come from.
> 

Eh? XTF still has lots of features which XBL2 doesn't

Comment 14 User image Doron Rosenberg (IBM) 2006-03-24 08:11:37 PST
(In reply to comment #13)
> (In reply to comment #12)
> > XTF is a dead end. XBL2 is where the solution should come from.
> > 
> 
> Eh? XTF still has lots of features which XBL2 doesn't
> 

Last I heard, they planned on adding xtf-like functionality into xbl2 :)
Comment 15 User image Allan Beaufour 2006-03-26 23:49:11 PST
(In reply to comment #13)
> (In reply to comment #12)
> > XTF is a dead end. XBL2 is where the solution should come from.
> > 
> 
> Eh? XTF still has lots of features which XBL2 doesn't

Ok, I was maybe being a bit short/cocky there :)

I do not believe that anybody (besides us) are going to extend or maintain XTF. Afaik, the plan is for XBL2 to solve the things that XTF solve. Moreover, 1.8.x is most probably closed land, so any enhancements to XTF would have to target 1.9 -- that is distant future.

imho, we should make sure that we can use XBL2 instead of XTF at some point in the future. Unfortunately, I have not had time to investigate XBL2 :(

More specifically for this bug, the best solution I have is the hack I've just detailed.
Comment 16 User image alexander :surkov 2006-04-28 00:57:19 PDT
1) XBL bindings aren't applied to html:td/html:tr elements.
2) Is Allan's hack supposes a support of nested attribute based repeats? F.x.

<xf:model><xf:instance>
  <root xmlns="">
    <item>
      <subitem>1.1</subitem>
      <subitem>1.2</subitem>
    </item>
    <item>
      <subitem>2.1</subitem>
      <subitem>2.2</subitem>
    </item>
  </root>
</xf:instance></xf:model>

<grid>
 <columns>
   <column/>
   <column/>
 </columns>
 <rows xf:repeat-nodeset="/bla/bla">
   <row xf:repeat-nodeset="*">
     <xf:output ref="."/>
   </row>
 </rows>
</grid>
Comment 17 User image Allan Beaufour 2006-05-04 05:27:27 PDT
(In reply to comment #16)
> 1) XBL bindings aren't applied to html:td/html:tr elements.

Are you saying that XBL bindings does not work for td/tr elements?

> 2) Is Allan's hack supposes a support of nested attribute based repeats?

It is supposed to work, and in theory it should. That said, it _is_ a hack.
Comment 18 User image Allan Beaufour 2006-06-01 09:28:08 PDT
Created attachment 224077 [details]
Testcase  - div
Comment 19 User image Allan Beaufour 2006-06-01 09:28:45 PDT
Created attachment 224078 [details]
Testcase  - div, nested
Comment 20 User image Allan Beaufour 2006-06-01 09:29:21 PDT
Created attachment 224079 [details]
Test - table (tr)
Comment 21 User image Allan Beaufour 2006-06-01 09:36:34 PDT
Created attachment 224080 [details] [diff] [review]
Work in progress

This is some of the way
* the approach is the one from comment 8
* I had to change element defering, because it was deferring on the XBL doc
* I had to do a lot of css changes, ie. make the xforms namespace explicit, or else the binding would not work. It might be bogus though...
* tr does not work. the binding is never triggered
Comment 22 User image Allan Beaufour 2006-06-02 02:07:46 PDT
Created attachment 224176 [details]
Testcase - div (including insert and delete)
Comment 23 User image Allan Beaufour 2006-06-02 02:10:59 PDT
Created attachment 224178 [details] [diff] [review]
Patch

Here's a cleaned up version of attachment 224080 [details] [diff] [review]. The css changes were bogus, otherwise this is just as noted in comment 21.
Comment 24 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2006-06-02 03:12:58 PDT
Comment on attachment 224178 [details] [diff] [review]
Patch


>+nsXFormsModelElement::DeferElementBind(nsIXFormsControl *aControl)
> {
>-  nsCOMPtr<nsIDocument> doc = do_QueryInterface(aDoc);
>+  NS_ENSURE_ARG_POINTER(aControl);
>+  nsCOMPtr<nsIDOMElement> element;
>+  nsresult rv = aControl->GetElement(getter_AddRefs(element));
>+  NS_ENSURE_SUCCESS(rv, rv);
>+  
>+  nsCOMPtr<nsIContent> content(do_QueryInterface(element));
>+  NS_ASSERTION(content, "nsIDOMElement not implementing nsIContent?!");
> 
>-  if (!doc || !aControl) {
>-    return NS_ERROR_FAILURE;
>+  nsCOMPtr<nsIDocument> doc = content->GetCurrentDoc();
>+  if (!doc) {
>+    // We do not carea about elements without a document. If they get added to
>+    // a document at some point in time, they'll try to bind again.
>+    return NS_OK;

typo "carea"


>+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));
>+  NS_ENSURE_STATE(domDoc);
>+
>+  nsCOMPtr<nsIDOMDocumentXBL> docXBL(do_QueryInterface(domDoc));
>+  NS_ENSURE_STATE(docXBL);


No need the check NS_ENSURE_STATE(domDoc), that will be done 
after QI to docXML

Looks great :) This is a bit strange way to use XBL, but it works.
r=me
Comment 25 User image Allan Beaufour 2006-06-02 03:24:16 PDT
(In reply to comment #24)
> This is a bit strange way to use XBL, but it works.

Is that not the purpose of extensions/xforms? Use XTF and XBL in strange and twisted ways ;-)
Comment 26 User image Allan Beaufour 2006-06-02 03:25:20 PDT
Created attachment 224184 [details] [diff] [review]
Patch with smaug's comments fixed
Comment 27 User image Doron Rosenberg (IBM) 2006-06-02 07:49:52 PDT
Comment on attachment 224184 [details] [diff] [review]
Patch with smaug's comments fixed

Not the most efficient at places (those removeAttribute calls, can't they be done on nsIContent faster?), but this is repeat, it always will be slow :)
Comment 28 User image Allan Beaufour 2006-06-06 07:08:46 PDT
Fixed on branch. It _does not_ work for tr elements.
Comment 29 User image aaronr 2006-06-06 10:28:46 PDT
(In reply to comment #28)
> Fixed on branch. It _does not_ work for tr elements.
> 


Could you make sure that this is readme'd or mentioned on the status page?  Otherwise we'll lose track of it.
Comment 30 User image aaronr 2006-06-26 15:35:38 PDT
(In reply to comment #28)
> Fixed on branch. It _does not_ work for tr elements.
> 

For those interested in why, read: https://bugzilla.mozilla.org/show_bug.cgi?id=83830
Comment 31 User image Allan Beaufour 2006-08-08 06:57:07 PDT
Are we still using 'xf-to-branch' mechanism, so this still needs to be sync'ed to branches?
Comment 32 User image aaronr 2006-08-08 10:11:07 PDT
(In reply to comment #31)
> Are we still using 'xf-to-branch' mechanism, so this still needs to be sync'ed
> to branches?
> 

yep, still needs to go to branches.
Comment 33 User image aaronr 2006-10-17 14:10:04 PDT
checked into 1.8.0 branch on 2006/09/21
Comment 34 User image aaronr 2007-01-11 15:41:11 PST
checked into 1.8 branch on 2006/11/21

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