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 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 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 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 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 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 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 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 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 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 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 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 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 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 Olli Pettay [:smaug] 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 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 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 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 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 Allan Beaufour 2006-06-01 09:28:08 PDT
Created attachment 224077 [details]
Testcase  - div
Comment 19 Allan Beaufour 2006-06-01 09:28:45 PDT
Created attachment 224078 [details]
Testcase  - div, nested
Comment 20 Allan Beaufour 2006-06-01 09:29:21 PDT
Created attachment 224079 [details]
Test - table (tr)
Comment 21 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 Allan Beaufour 2006-06-02 02:07:46 PDT
Created attachment 224176 [details]
Testcase - div (including insert and delete)
Comment 23 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 Olli Pettay [:smaug] 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 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 Allan Beaufour 2006-06-02 03:25:20 PDT
Created attachment 224184 [details] [diff] [review]
Patch with smaug's comments fixed
Comment 27 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 Allan Beaufour 2006-06-06 07:08:46 PDT
Fixed on branch. It _does not_ work for tr elements.
Comment 29 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 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 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 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 aaronr 2006-10-17 14:10:04 PDT
checked into 1.8.0 branch on 2006/09/21
Comment 34 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.