Behaviors implementing IWiQueryPlugin are not rendered in ajax requests

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Create a page, a component and a behaviour. 
2. Add the component to the page, don't let the component implement 
IWiQueryPlugin.
3. Add the behaviour to the component, let the behaviour implement 
IWiQueryPlugin.
4. Create an ajax request and add the component to it.

What is the expected output? What do you see instead?
The WiQueryCoreHeaderContributor is not added. I expected the 
WiQueryCoreHeaderContributor to add the wiquery code.

What version of the product are you using? On what operating system?
wicket 1.4.12, wiquery 1.1-alpha.

Please provide any additional information below.
http://github.com/papegaaij/wiquery/commit/10204e0d976cd12ef4e1ca7dfab0389db59fc
0fc

The above link shows a patch to add the WiQueryCoreHeaderContributor as an 
IComponentOnBeforeRenderListener (as well as an 
IComponentInstantiationListener) and let the WiQueryCoreHeaderContributor check 
the components for behaviours that are IWiQueryPlugins.

Original issue reported on code.google.com by hielke.hoeve on 27 Sep 2010 at 3:03

[GoogleCodeExporter]

Note that commit 
http://github.com/papegaaij/wiquery/commit/cf78a613220b1feb9bda71c7af1790fe3631e
d22 is a small performance tweak.

Original comment by hielke.hoeve on 27 Sep 2010 at 3:04

[GoogleCodeExporter]

Are you extending WiQueryAbstractBehavior? 
WiQueryAbstractBehavior does the necessary logic to ensure that the 
WiQueryCoreHeaderContributor is added to the component.  
If you do not extend WiQueryAbstractBehavior you can get the same effect by 
having this in your IBehavior impl:

@Override
public void bind(Component component) {
    component.add(new HeaderContributor(new WiQueryCoreHeaderContributor()));
    super.bind(component);
}

-- 
Regards - Richard Wilkinson
Developer,
jWeekend: OO & Java Technologies - Development and Training
http://jWeekend.com

Original comment by richardj...@gmail.com on 29 Sep 2010 at 11:00

[GoogleCodeExporter]

The mentioned patches provide a solution without WiQueryAbstractBehavior. All 
behaviors implementing IWiQueryPlugin are automatically detected.

Original comment by emond.pa...@gmail.com on 29 Sep 2010 at 11:47

[GoogleCodeExporter]

The problem is bigger than it first seemed. 

Component which are added during an ajax call are not handled by the 
WiQueryCoreHeaderContributor. IWiQueryPlugin Behaviours which are added to 
non-IWiQueryPlugin components are not added at any time.

Our patches
http://github.com/papegaaij/wiquery/commit/9207568c892cca1d29964b8efe479a9660578
dd2
http://github.com/papegaaij/wiquery/commit/5fe210377d968fcf64d236efec6a8e7972d83
95f
http://github.com/papegaaij/wiquery/commit/8a4dc8c9ae863c68a8b03624ae3328f12fbae
7de

fix the issue 60 problem and this problem. This also eliminates the need for 
WiQueryAbstractBehavior and AbstractDefaultAjaxBehavior, because the 
WiQueryCoreHeaderContributor handles IWiQueryPlugin behaviours and/or 
components.
Since java does not support multiple inheritance using an annotation is the 
easiest way.

Original comment by hielke.hoeve on 29 Sep 2010 at 11:56

[GoogleCodeExporter]

But a lot of plugins use these two classes (internal and external). So, we have 
to preserve these. And we have to check if the modification will execute one or 
twice times the WiQueryAbstractBehavior.

I will look on this recent listener (since 1.4.9) and try to include this 
enhancement for the 1.1.

Regards

Julien Roche

Original comment by roche....@gmail.com on 29 Sep 2010 at 11:59

• Added labels: Type-Enhancement, Milestone-Release1.1
• Removed labels: Type-Defect

[GoogleCodeExporter]

It's always possible to make then @deprecated and simply use them as tagging 
classes (no body).

Thanks for checking it out.

Original comment by hielke.hoeve on 29 Sep 2010 at 12:03

[GoogleCodeExporter]

Components added on ajax request should be rendered, but it seems there is a 
small bug in your patch, component.setMetaData(WI_QUERY_RENDERED, 
Boolean.TRUE); should only happen inside the if statement checking the type of 
the behaviour, otherwise it will be marked as including the core contributor on 
the first render even if it doesn't.

I do like the idea of using an IComponentOnBeforeRenderListener instead of 
IComponentInstantiationListener, as this allows us to check for any IBehavior 
implementing IWiQueryPlugin and for components implementing IWiQueryPlugin 
without requiring any extra coding from the user.

We could make this change transparently by putting it into the 
WiQueryInitializer, which gets run for all users.  We can then remove the bit 
of code in WiQueryAbstractBehavior that I listed earlier.

I didn't look through all of the patches though, why does 
WiQueryCoreHeaderContributor need to have an handle on the component?

Patch is to be applied to trunk (not committed), will need more testing though.

-- 
Regards - Richard Wilkinson
Developer,
jWeekend: OO & Java Technologies - Development and Training
http://jWeekend.com

Original comment by richardj...@gmail.com on 29 Sep 2010 at 12:44

Attachments:

• patch.txt

[GoogleCodeExporter]

WiQueryCoreHeaderContributor needs an handle when the request is an ajax 
request.

Original comment by hielke.hoeve on 29 Sep 2010 at 12:48

[GoogleCodeExporter]

do you have a quickstart? i didnt need to modify the 
WiQueryCoreHeaderContributor, after using the patch I attached (based on your 
patch)

The previous WiQueryCoreHeaderContributor loops through all the components on 
the ART, so should be able to find any components / behaviours that need jQuery 
adding, but maybe there is something I have overlooked?

I have attached the quickstart I am using for testing if that helps.

-- 
Regards - Richard Wilkinson
Developer,
jWeekend: OO & Java Technologies - Development and Training
http://jWeekend.com

Original comment by richardj...@gmail.com on 29 Sep 2010 at 1:17

Attachments:

• mycompany.zip

[GoogleCodeExporter]

The last patch does a whole lot more than the first two. We discovered an issue 
with WiQueryCoreHeaderContributor with AJAX requests. On the first component in 
an AJAX request, the WiQueryCoreHeaderContributor would loop through all 
components to find IWiQueryPlugins. However, if one of those components were to 
change the component hierarchy in onBeforeRender (like with ListViews), these 
components would not be added as IWiQueryPlugins.

We completely rewrote/reorganized WiQueryCoreHeaderContributor to use a 
different approach for AJAX requests. In a normal request, it renders all 
JsStatements in one script, but for AJAX requests, it only renders the 
JsStatement for the owner component. For AJAX it doesn't really matter to merge 
the js anyway.

The WI_QUERY_RENDERED check is there to save resources. A component will only 
be scanned for IWiQueryPlugin behaviors once. This does however have the 
drawback that behaviors added in a second request will not be detected. The 
marker could be placed inside the loop. I don't think it will make that much of 
a difference.

We've applied these patches to all of our applications and are pushing it to 
production today. I think they are quite solid.

Original comment by emond.pa...@gmail.com on 29 Sep 2010 at 1:24

[GoogleCodeExporter]

We don't have a quickstart, but the setup is easy: have a page with 2 
components, a IWiQueryPlugin (stand-alone) and a ListView with an 
IWiQueryPlugin on every row. Add a button that adds a row to the model of the 
ListView and add the stand-alone component and the ListView to the ART (in that 
order, and ofcourse you will need to add a WMC because you can't add ListViews 
to ART).

The will cause the components to be scanned before the onBeforeRender of the 
ListView is called. ListViews create rows in the onBeforeRender, so the newly 
added row will not have its js rendered.

Original comment by emond.pa...@gmail.com on 29 Sep 2010 at 1:35

[GoogleCodeExporter]

ah ok, I hadn't accounted for that. AFAIK, all the onBeforeRenders are called 
before the renderHeads are called in a normal page render, and I had assumed it 
would be the same with ajax (may be wrong here too though).

Original comment by richardj...@gmail.com on 29 Sep 2010 at 1:54

[GoogleCodeExporter]

For ajax, every component is rendered in its own 'phase'. So it will be: head1, 
onBeforeRender1, head2, onBeforeRender2, etc. You can see this in the code for 
ART.

Original comment by emond.pa...@gmail.com on 29 Sep 2010 at 2:11

[GoogleCodeExporter]

Yea i looked up the code after your last comment :)

Ok, I've added your patches, with one slight change, i've moved the  
component.setMetaData(WI_QUERY_RENDERED, true); to inside the 
WiQueryCoreHeaderContributor constructor, this way if anyone is manually adding 
the WiQueryCoreHeaderContributor in any of their code it wont get added twice 
by the before render listener and also it means that if behaviours are added on 
later requests they will still be included (is the same as moving it inside the 
if statement)

I've committed it into a branch at the moment /branches/issue-105, because it 
also changes how the initialiser and WiQueryApplication is used, and some other 
members of the team will probably have views on that.

-- 
Regards - Richard Wilkinson
Developer,
jWeekend: OO & Java Technologies - Development and Training
http://jWeekend.com

Original comment by richardj...@gmail.com on 29 Sep 2010 at 2:37

[GoogleCodeExporter]

Hi richard,

I will try / inject / adapt your code into the trunk and I will test it tonight 
(or tomorrow or this weekend :'(  )

Thanks a lot

Cheers

Julien Roche

Original comment by roche....@gmail.com on 29 Sep 2010 at 4:17

[GoogleCodeExporter]

Merged the branch (/branches/issue-105) into trunk

Original comment by richardj...@gmail.com on 30 Sep 2010 at 9:35

• Changed state: Fixed

[GoogleCodeExporter]

Perfect

Thank you Richard

Julien Roche

Original comment by roche....@gmail.com on 30 Sep 2010 at 11:59