returnListener is broken

Hi,

if i add a returnListener to my commandButton, which is
defined in a facet from a table, the returnListener is not
called anymore. He is a short example:

<tr:table var="row"
value="#{backing.allRows}">
  <f:facet name="footer">
    <tr:panelButtonBar>
      <tr:commandButton text="Hit me"
partialSubmit="true" useWindow="true"
immediate="true"
action="dialog:openHitMeDialog"
returnListener="#{backing.handleReturn}" />
    </tr:panelButtonBar>
  </f:facet>
  <tr:column>
    <f:facet name="header">
      <af:outputText value="Header 1"
    </f:facet>
    <tr:outputText value="#{row.data}" />
  </tr:column>
</tr:table>

I have tested this scenario with trinidad 1.0.2 and it works
fine. The difference between these releases is amongst
others the jira issue TRINIDAD-737. I have removed this
changes from my trinidad 1.0.3 source and started a new
build and now the returnListener is called again.
Is it a problem how i defined the commandButton in my page
or is it a bug that comes with TRINIDAD-737???

Hope anybody can help me...

Cheers Sven
-- 
Psssst! Schon vom neuen GMX MultiMessenger gehört?
Der kanns mit allen: http://www.gm
x.net/de/go/multimessenger

OK, it's not a general issue with returnListener;  it's
returnListener
inside a facet of the table.  Nice job analyzing this down
to
TRINIDAD-737 - makes it easy to discern the problem.

The problem is that we've started setting currency by
calling
setRowKey() on the table, but that means that the clientId
of
the contents start changing...  which is wrong.   Shoulda
thought
of that when I reviewed this.  There'll be other things that
break
(for instance, PPR within a facet of the table).

The fix, I think, is to not set the currency, but instead
merely
set up the EL state as if currency were set, e.g. going
from
  - set row key to first selected row
  - process the event
  - restore row key

... to the more verbose:

  - get original values "var" and
"varStatus" off the request map
  - set row key to first selected row
  - grab new "var" and "varStatus" off
the request map
  - restore row key
  - set new "var" and "varStatus" onto
the request map
  - process the event
  - restore "var" and "varStatus" to
original values

For now, you can work around this (significant) bug by
moving the panelButtonBar to after the table, and
out of the "footer".  But we should roll a 1.0.4
fairly
soon to fix this regression, and any others if they get
found soon.

-- Adam



On 10/8/07, junglemengmx.de <junglemengmx.de> wrote:
> Hi,
>
> if i add a returnListener to my commandButton, which is
defined in a facet from a table, the returnListener is not
called anymore. He is a short example:
>
> <tr:table var="row"
value="#{backing.allRows}">
>   <f:facet name="footer">
>     <tr:panelButtonBar>
>       <tr:commandButton text="Hit me"
partialSubmit="true" useWindow="true"
immediate="true"
action="dialog:openHitMeDialog"
returnListener="#{backing.handleReturn}" />
>     </tr:panelButtonBar>
>   </f:facet>
>   <tr:column>
>     <f:facet name="header">
>       <af:outputText value="Header 1"
>     </f:facet>
>     <tr:outputText value="#{row.data}"
/>
>   </tr:column>
> </tr:table>
>
> I have tested this scenario with trinidad 1.0.2 and it
works fine. The difference between these releases is amongst
others the jira issue TRINIDAD-737. I have removed this
changes from my trinidad 1.0.3 source and started a new
build and now the returnListener is called again.
> Is it a problem how i defined the commandButton in my
page or is it a bug that comes with TRINIDAD-737???
>
> Hope anybody can help me...
>
> Cheers Sven
>
> --
> Psssst! Schon vom neuen GMX MultiMessenger gehört?
> Der kanns mit allen: http://www.gm
x.net/de/go/multimessenger
>

Thanks, Adam.

I like your suggestion. The only exception I see is that we
would still
establish currency for selection events. Agreed?

I will submit a new patch for review. Sorry for introducing
the bug in a
first place.

Max



Adam Winer wrote:
> 
> 
> The fix, I think, is to not set the currency, but
instead merely
> set up the EL state as if currency were set, e.g. going
from
>   - set row key to first selected row
>   - process the event
>   - restore row key
> 
> ... to the more verbose:
> 
>   - get original values "var" and
"varStatus" off the request map
>   - set row key to first selected row
>   - grab new "var" and "varStatus"
off the request map
>   - restore row key
>   - set new "var" and "varStatus"
onto the request map
>   - process the event
>   - restore "var" and "varStatus"
to original values
> 
> For now, you can work around this (significant) bug by
> moving the panelButtonBar to after the table, and
> out of the "footer".  But we should roll a
1.0.4 fairly
> soon to fix this regression, and any others if they
get
> found soon.
> 
> 
> 

-- 
View this message in context: http://www.nabble.com/-T
rinidad-1.0.3--returnListener-is-broken-tf4594019.html#a1325
4310
Sent from the MyFaces - Users mailing list archive at
Nabble.com.

We will not be able to  establish varStatus actually because
the object we
normally store on a request map refers to the collection
component and
expects it to have the right currency:

Max
-- 
View this message in context: http://www.nabble.com/-T
rinidad-1.0.3--returnListener-is-broken-tf4594019.html#a1325
4959
Sent from the MyFaces - Users mailing list archive at
Nabble.com.

On 10/17/07, max.starets <Max.Staretsoracle.com> wrote:
>
> Thanks, Adam.
>
> I like your suggestion. The only exception I see is
that we would still
> establish currency for selection events. Agreed?

Wouldn't that still mean that if you launch a dialog during
a selection
event, the return will fail?.  Or that PPR'ing a facet of
the table in response
to a selection event will likely fail?

I'm wondering if we want something much uglier here:  a way
to
still establish currency (and do it for all events), but
without
mucking with client IDs.

-- Adam


>
> I will submit a new patch for review. Sorry for
introducing the bug in a
> first place.
>
> Max
>
>
>
> Adam Winer wrote:
> >
> >
> > The fix, I think, is to not set the currency, but
instead merely
> > set up the EL state as if currency were set, e.g.
going from
> >   - set row key to first selected row
> >   - process the event
> >   - restore row key
> >
> > ... to the more verbose:
> >
> >   - get original values "var" and
"varStatus" off the request map
> >   - set row key to first selected row
> >   - grab new "var" and
"varStatus" off the request map
> >   - restore row key
> >   - set new "var" and
"varStatus" onto the request map
> >   - process the event
> >   - restore "var" and
"varStatus" to original values
> >
> > For now, you can work around this (significant)
bug by
> > moving the panelButtonBar to after the table, and
> > out of the "footer".  But we should roll
a 1.0.4 fairly
> > soon to fix this regression, and any others if
they get
> > found soon.
> >
> >
> >
>
> --
> View this message in context: http://www.nabble.com/-T
rinidad-1.0.3--returnListener-is-broken-tf4594019.html#a1325
4310
> Sent from the MyFaces - Users mailing list archive at
Nabble.com.
>
>

Good point, Adam.

I submitted a new patch for JIRA-771 that completely reverts
changes made
for JIRA-737.

Regarding concerns raised in JIRA-737: would it make sense
to provide an EL
variable
for selected row (add a "selectedVar" property for
table)? This would allow
declarative
code to refer to the selected row (which I always thought
was lacking in
table) without 
us having to do something ugly with the client IDs.

Thanks,
Max


Adam Winer wrote:
> 
> On 10/17/07, max.starets <Max.Staretsoracle.com> wrote:
>>
>> Thanks, Adam.
>>
>> I like your suggestion. The only exception I see is
that we would still
>> establish currency for selection events. Agreed?
> 
> Wouldn't that still mean that if you launch a dialog
during a selection
> event, the return will fail?.  Or that PPR'ing a facet
of the table in
> response
> to a selection event will likely fail?
> 
> I'm wondering if we want something much uglier here:  a
way to
> still establish currency (and do it for all events),
but without
> mucking with client IDs.
> 
> -- Adam
> 
> 

-- 
View this message in context: http://www.nabble.com/-T
rinidad-1.0.3--returnListener-is-broken-tf4594019.html#a1336
5888
Sent from the MyFaces - Users mailing list archive at
Nabble.com.

Interesting idea - I could go for that...  Not 100% thrilled
about
adding a new EL variable, but OTOH I could also question why
we'd
ever want varStatus set to the selected row (which happens
if you
tie currency to selection).

-- Adam

On 10/23/07, max.starets <Max.Staretsoracle.com> wrote:
>
> Good point, Adam.
>
> I submitted a new patch for JIRA-771 that completely
reverts changes made
> for JIRA-737.
>
> Regarding concerns raised in JIRA-737: would it make
sense to provide an EL
> variable
> for selected row (add a "selectedVar"
property for table)? This would allow
> declarative
> code to refer to the selected row (which I always
thought was lacking in
> table) without
> us having to do something ugly with the client IDs.
>
> Thanks,
> Max
>
>
> Adam Winer wrote:
> >
> > On 10/17/07, max.starets <Max.Staretsoracle.com> wrote:
> >>
> >> Thanks, Adam.
> >>
> >> I like your suggestion. The only exception I
see is that we would still
> >> establish currency for selection events.
Agreed?
> >
> > Wouldn't that still mean that if you launch a
dialog during a selection
> > event, the return will fail?.  Or that PPR'ing a
facet of the table in
> > response
> > to a selection event will likely fail?
> >
> > I'm wondering if we want something much uglier
here:  a way to
> > still establish currency (and do it for all
events), but without
> > mucking with client IDs.
> >
> > -- Adam
> >
> >
>
> --
> View this message in context: http://www.nabble.com/-T
rinidad-1.0.3--returnListener-is-broken-tf4594019.html#a1336
5888
> Sent from the MyFaces - Users mailing list archive at
Nabble.com.
>
>