You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@shindig.apache.org by "James McIninch (JIRA)" <ji...@apache.org> on 2010/01/11 15:38:54 UTC

[jira] Created: (SHINDIG-1260) Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors

Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors
----------------------------------------------------------------------------------------------------

                 Key: SHINDIG-1260
                 URL: https://issues.apache.org/jira/browse/SHINDIG-1260
             Project: Shindig
          Issue Type: Bug
          Components: PHP
    Affects Versions: 1.1-BETA5, 1.1-BETA6, 1.1-RC1
         Environment: RedHat Linux 5.3, Apache 2.2.3, PHP 5.2.10
            Reporter: James McIninch


The logic for selecting and inserting JavaScript using the GadgetFeatureRegistry and GadgetBaseRender is incorrect. The result is that JavaScript for features is included out-of-order of their dependencies, sometimes multiply included, and the 'gadgets' JavaScript object is not defined.

Consider the following simple gadget:

<?xml version="1.0" encoding="UTF-8"?>
<Module>
	<ModulePrefs title="Hello" description="An example of a simple gadget" author="James McIninch" author_email="james.mcininc@biogenidec.com" />
	<UserPref name="firstname" display_name="Your name" datatype="string" default_value="stranger" />
	<Content type="html" preferred_height="50" preferred_width="200">
		<![CDATA[
		    <script type="text/javascript">
		       var prefs = new gadgets.Prefs();
		       var name  = prefs.getString("firstname");
		       document.getElementById("placeholder").innerHTML = name;
		    </script>
		    
		    Hello, <span id="placeholder">name goes here</span>!
		]]>
	</Content>
</Module>

This is rendered as:

Hello, name goes here!

Inspecting the JavaScript error console shows that 'gadgets' is undefined. However, the first instance of 'gadgets' being undefined lies in the Shindig-included JavaScript. This is because auth-init.js is included before config.js (both in features/src/main/javascript/features/core.*), but auth-init.js contains the line (28):

shindig.auth = new shindig.Auth();

which, in turn calls (auth.js line 159):

gadgets.config.register("shindig.auth", null, init);

... but the 'gadgets' and 'gadgets.config' objects are not yet defined at that point. Despite being dependencies for the auth.js, they aren't  included in the order where the dependencies precede the code that depends on them.

Also looking at the source of the rendered gadget, the taming.js code is included 5 separate times.

Correcting the bug will entail re-thinking the logic of the feature ordering to include the dependency information and order the features based on the dependencies.

All gadgets requiring the 'gadgets' object to be defined currently fail. Gadgets using the legacy model still function, though there exists redundant and erroneous JavaScript.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SHINDIG-1260) Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors

Posted by "Jacky Wang (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SHINDIG-1260?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jacky Wang updated SHINDIG-1260:
--------------------------------

    Attachment:     (was: shindig-1260-jacky.patch)

> Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors
> ----------------------------------------------------------------------------------------------------
>
>                 Key: SHINDIG-1260
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-1260
>             Project: Shindig
>          Issue Type: Bug
>          Components: PHP
>    Affects Versions: 1.1-BETA5, 1.1-BETA6, 1.1-RC1
>         Environment: RedHat Linux 5.3, Apache 2.2.3, PHP 5.2.10
>            Reporter: James McIninch
>         Attachments: shindig-1260.patch
>
>
> The logic for selecting and inserting JavaScript using the GadgetFeatureRegistry and GadgetBaseRender is incorrect. The result is that JavaScript for features is included out-of-order of their dependencies, sometimes multiply included, and the 'gadgets' JavaScript object is not defined.
> Consider the following simple gadget:
> <?xml version="1.0" encoding="UTF-8"?>
> <Module>
> 	<ModulePrefs title="Hello" description="An example of a simple gadget" author="James McIninch" author_email="james.mcininc@biogenidec.com" />
> 	<UserPref name="firstname" display_name="Your name" datatype="string" default_value="stranger" />
> 	<Content type="html" preferred_height="50" preferred_width="200">
> 		<![CDATA[
> 		    <script type="text/javascript">
> 		       var prefs = new gadgets.Prefs();
> 		       var name  = prefs.getString("firstname");
> 		       document.getElementById("placeholder").innerHTML = name;
> 		    </script>
> 		    
> 		    Hello, <span id="placeholder">name goes here</span>!
> 		]]>
> 	</Content>
> </Module>
> This is rendered as:
> Hello, name goes here!
> Inspecting the JavaScript error console shows that 'gadgets' is undefined. However, the first instance of 'gadgets' being undefined lies in the Shindig-included JavaScript. This is because auth-init.js is included before config.js (both in features/src/main/javascript/features/core.*), but auth-init.js contains the line (28):
> shindig.auth = new shindig.Auth();
> which, in turn calls (auth.js line 159):
> gadgets.config.register("shindig.auth", null, init);
> ... but the 'gadgets' and 'gadgets.config' objects are not yet defined at that point. Despite being dependencies for the auth.js, they aren't  included in the order where the dependencies precede the code that depends on them.
> Also looking at the source of the rendered gadget, the taming.js code is included 5 separate times.
> Correcting the bug will entail re-thinking the logic of the feature ordering to include the dependency information and order the features based on the dependencies.
> All gadgets requiring the 'gadgets' object to be defined currently fail. Gadgets using the legacy model still function, though there exists redundant and erroneous JavaScript.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SHINDIG-1260) Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors

Posted by "Jacky Wang (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SHINDIG-1260?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jacky Wang updated SHINDIG-1260:
--------------------------------

    Attachment:     (was: shindig-1260-jacky-final.patch)

> Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors
> ----------------------------------------------------------------------------------------------------
>
>                 Key: SHINDIG-1260
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-1260
>             Project: Shindig
>          Issue Type: Bug
>          Components: PHP
>    Affects Versions: 1.1-BETA5, 1.1-BETA6, 1.1-RC1
>         Environment: RedHat Linux 5.3, Apache 2.2.3, PHP 5.2.10
>            Reporter: James McIninch
>         Attachments: shindig-1260.patch
>
>
> The logic for selecting and inserting JavaScript using the GadgetFeatureRegistry and GadgetBaseRender is incorrect. The result is that JavaScript for features is included out-of-order of their dependencies, sometimes multiply included, and the 'gadgets' JavaScript object is not defined.
> Consider the following simple gadget:
> <?xml version="1.0" encoding="UTF-8"?>
> <Module>
> 	<ModulePrefs title="Hello" description="An example of a simple gadget" author="James McIninch" author_email="james.mcininc@biogenidec.com" />
> 	<UserPref name="firstname" display_name="Your name" datatype="string" default_value="stranger" />
> 	<Content type="html" preferred_height="50" preferred_width="200">
> 		<![CDATA[
> 		    <script type="text/javascript">
> 		       var prefs = new gadgets.Prefs();
> 		       var name  = prefs.getString("firstname");
> 		       document.getElementById("placeholder").innerHTML = name;
> 		    </script>
> 		    
> 		    Hello, <span id="placeholder">name goes here</span>!
> 		]]>
> 	</Content>
> </Module>
> This is rendered as:
> Hello, name goes here!
> Inspecting the JavaScript error console shows that 'gadgets' is undefined. However, the first instance of 'gadgets' being undefined lies in the Shindig-included JavaScript. This is because auth-init.js is included before config.js (both in features/src/main/javascript/features/core.*), but auth-init.js contains the line (28):
> shindig.auth = new shindig.Auth();
> which, in turn calls (auth.js line 159):
> gadgets.config.register("shindig.auth", null, init);
> ... but the 'gadgets' and 'gadgets.config' objects are not yet defined at that point. Despite being dependencies for the auth.js, they aren't  included in the order where the dependencies precede the code that depends on them.
> Also looking at the source of the rendered gadget, the taming.js code is included 5 separate times.
> Correcting the bug will entail re-thinking the logic of the feature ordering to include the dependency information and order the features based on the dependencies.
> All gadgets requiring the 'gadgets' object to be defined currently fail. Gadgets using the legacy model still function, though there exists redundant and erroneous JavaScript.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SHINDIG-1260) Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors

Posted by "Jacky Wang (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SHINDIG-1260?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jacky Wang updated SHINDIG-1260:
--------------------------------

    Attachment: shindig-1260-final.patch

Hi All,

This patch is complete now.

Basically, it fixes the issue SHINDIG-1260, which could be found here:
http://issues.apache.org/jira/browse/SHINDIG-1260

The code-review could be found here: http://codereview.appspot.com/194059/show.

In details, this patch does the following things:

1. Change all the name "focedJsLibs" to "forcedJsLibs".

2. Sort the order of features according to their dependencies.

3. If the forced-javascript-lib is not included in the required features, it won't be included in the generated gadget, and it won't be appear in the gadget.init, either.

4. JsServlet's behaviors is changed: it won't expand the javascript content according to the dependency anymore.  This prevent the redundant inclusion of the js content.

5. The js generating logic is changed:
   Given a dependency graph inl_A <- inl_B <- fjl_C <- fjl_D <- inl_E <- fjl_F (inl = inline, fjl = forcedJsLib):
   The existing solution produces [fjl: C+D+F], [inline A+B+E] in the header/body.  However, it breaks the dependency graph.
   In contrast, this new patch groups the features into groups, and each one belongs to either "inlined" or "forced external".
   Therefore the example yields the following groups: [inl_A, inl_B], [fjl_C, fjl_D], [inl_E], [fjl_F] and generates the proper code in header/body.

6. Since the dependency graph might generate different topological sorted orders, heuristic method is used to make the feature group number as less as possible, thus reduces the outbound request number of javascript content.

I've tested it against many gadgets, and it works quite well.

Hope it helps.

Cheers,
Jacky

> Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors
> ----------------------------------------------------------------------------------------------------
>
>                 Key: SHINDIG-1260
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-1260
>             Project: Shindig
>          Issue Type: Bug
>          Components: PHP
>    Affects Versions: 1.1-BETA5, 1.1-BETA6, 1.1-RC1
>         Environment: RedHat Linux 5.3, Apache 2.2.3, PHP 5.2.10
>            Reporter: James McIninch
>         Attachments: shindig-1260-final.patch, shindig-1260.patch
>
>
> The logic for selecting and inserting JavaScript using the GadgetFeatureRegistry and GadgetBaseRender is incorrect. The result is that JavaScript for features is included out-of-order of their dependencies, sometimes multiply included, and the 'gadgets' JavaScript object is not defined.
> Consider the following simple gadget:
> <?xml version="1.0" encoding="UTF-8"?>
> <Module>
> 	<ModulePrefs title="Hello" description="An example of a simple gadget" author="James McIninch" author_email="james.mcininc@biogenidec.com" />
> 	<UserPref name="firstname" display_name="Your name" datatype="string" default_value="stranger" />
> 	<Content type="html" preferred_height="50" preferred_width="200">
> 		<![CDATA[
> 		    <script type="text/javascript">
> 		       var prefs = new gadgets.Prefs();
> 		       var name  = prefs.getString("firstname");
> 		       document.getElementById("placeholder").innerHTML = name;
> 		    </script>
> 		    
> 		    Hello, <span id="placeholder">name goes here</span>!
> 		]]>
> 	</Content>
> </Module>
> This is rendered as:
> Hello, name goes here!
> Inspecting the JavaScript error console shows that 'gadgets' is undefined. However, the first instance of 'gadgets' being undefined lies in the Shindig-included JavaScript. This is because auth-init.js is included before config.js (both in features/src/main/javascript/features/core.*), but auth-init.js contains the line (28):
> shindig.auth = new shindig.Auth();
> which, in turn calls (auth.js line 159):
> gadgets.config.register("shindig.auth", null, init);
> ... but the 'gadgets' and 'gadgets.config' objects are not yet defined at that point. Despite being dependencies for the auth.js, they aren't  included in the order where the dependencies precede the code that depends on them.
> Also looking at the source of the rendered gadget, the taming.js code is included 5 separate times.
> Correcting the bug will entail re-thinking the logic of the feature ordering to include the dependency information and order the features based on the dependencies.
> All gadgets requiring the 'gadgets' object to be defined currently fail. Gadgets using the legacy model still function, though there exists redundant and erroneous JavaScript.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SHINDIG-1260) Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors

Posted by "Jacky Wang (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SHINDIG-1260?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12804550#action_12804550 ] 

Jacky Wang commented on SHINDIG-1260:
-------------------------------------

The first patch is made, as the attachment.

codereview here: http://codereview.appspot.com/194059/show

> Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors
> ----------------------------------------------------------------------------------------------------
>
>                 Key: SHINDIG-1260
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-1260
>             Project: Shindig
>          Issue Type: Bug
>          Components: PHP
>    Affects Versions: 1.1-BETA5, 1.1-BETA6, 1.1-RC1
>         Environment: RedHat Linux 5.3, Apache 2.2.3, PHP 5.2.10
>            Reporter: James McIninch
>         Attachments: shindig-1260-jacky.patch, shindig-1260.patch
>
>
> The logic for selecting and inserting JavaScript using the GadgetFeatureRegistry and GadgetBaseRender is incorrect. The result is that JavaScript for features is included out-of-order of their dependencies, sometimes multiply included, and the 'gadgets' JavaScript object is not defined.
> Consider the following simple gadget:
> <?xml version="1.0" encoding="UTF-8"?>
> <Module>
> 	<ModulePrefs title="Hello" description="An example of a simple gadget" author="James McIninch" author_email="james.mcininc@biogenidec.com" />
> 	<UserPref name="firstname" display_name="Your name" datatype="string" default_value="stranger" />
> 	<Content type="html" preferred_height="50" preferred_width="200">
> 		<![CDATA[
> 		    <script type="text/javascript">
> 		       var prefs = new gadgets.Prefs();
> 		       var name  = prefs.getString("firstname");
> 		       document.getElementById("placeholder").innerHTML = name;
> 		    </script>
> 		    
> 		    Hello, <span id="placeholder">name goes here</span>!
> 		]]>
> 	</Content>
> </Module>
> This is rendered as:
> Hello, name goes here!
> Inspecting the JavaScript error console shows that 'gadgets' is undefined. However, the first instance of 'gadgets' being undefined lies in the Shindig-included JavaScript. This is because auth-init.js is included before config.js (both in features/src/main/javascript/features/core.*), but auth-init.js contains the line (28):
> shindig.auth = new shindig.Auth();
> which, in turn calls (auth.js line 159):
> gadgets.config.register("shindig.auth", null, init);
> ... but the 'gadgets' and 'gadgets.config' objects are not yet defined at that point. Despite being dependencies for the auth.js, they aren't  included in the order where the dependencies precede the code that depends on them.
> Also looking at the source of the rendered gadget, the taming.js code is included 5 separate times.
> Correcting the bug will entail re-thinking the logic of the feature ordering to include the dependency information and order the features based on the dependencies.
> All gadgets requiring the 'gadgets' object to be defined currently fail. Gadgets using the legacy model still function, though there exists redundant and erroneous JavaScript.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SHINDIG-1260) Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors

Posted by "Jacky Wang (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SHINDIG-1260?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jacky Wang updated SHINDIG-1260:
--------------------------------

    Attachment: shindig-1260-jacky-final.patch

1) feature list sorted.
2) un-used features are removed from forcedJsLibs
3) re-factored Gadget*Renderer.php
4) don't expend the js on JsServlet.

> Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors
> ----------------------------------------------------------------------------------------------------
>
>                 Key: SHINDIG-1260
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-1260
>             Project: Shindig
>          Issue Type: Bug
>          Components: PHP
>    Affects Versions: 1.1-BETA5, 1.1-BETA6, 1.1-RC1
>         Environment: RedHat Linux 5.3, Apache 2.2.3, PHP 5.2.10
>            Reporter: James McIninch
>         Attachments: shindig-1260-jacky-final.patch, shindig-1260.patch
>
>
> The logic for selecting and inserting JavaScript using the GadgetFeatureRegistry and GadgetBaseRender is incorrect. The result is that JavaScript for features is included out-of-order of their dependencies, sometimes multiply included, and the 'gadgets' JavaScript object is not defined.
> Consider the following simple gadget:
> <?xml version="1.0" encoding="UTF-8"?>
> <Module>
> 	<ModulePrefs title="Hello" description="An example of a simple gadget" author="James McIninch" author_email="james.mcininc@biogenidec.com" />
> 	<UserPref name="firstname" display_name="Your name" datatype="string" default_value="stranger" />
> 	<Content type="html" preferred_height="50" preferred_width="200">
> 		<![CDATA[
> 		    <script type="text/javascript">
> 		       var prefs = new gadgets.Prefs();
> 		       var name  = prefs.getString("firstname");
> 		       document.getElementById("placeholder").innerHTML = name;
> 		    </script>
> 		    
> 		    Hello, <span id="placeholder">name goes here</span>!
> 		]]>
> 	</Content>
> </Module>
> This is rendered as:
> Hello, name goes here!
> Inspecting the JavaScript error console shows that 'gadgets' is undefined. However, the first instance of 'gadgets' being undefined lies in the Shindig-included JavaScript. This is because auth-init.js is included before config.js (both in features/src/main/javascript/features/core.*), but auth-init.js contains the line (28):
> shindig.auth = new shindig.Auth();
> which, in turn calls (auth.js line 159):
> gadgets.config.register("shindig.auth", null, init);
> ... but the 'gadgets' and 'gadgets.config' objects are not yet defined at that point. Despite being dependencies for the auth.js, they aren't  included in the order where the dependencies precede the code that depends on them.
> Also looking at the source of the rendered gadget, the taming.js code is included 5 separate times.
> Correcting the bug will entail re-thinking the logic of the feature ordering to include the dependency information and order the features based on the dependencies.
> All gadgets requiring the 'gadgets' object to be defined currently fail. Gadgets using the legacy model still function, though there exists redundant and erroneous JavaScript.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SHINDIG-1260) Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors

Posted by "Jacky Wang (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SHINDIG-1260?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jacky Wang updated SHINDIG-1260:
--------------------------------

    Attachment: shindig-1260-jacky.patch

1) feature list sorted.
2) un-used features are removed from forcedJsLibs
3) re-factored Gadget*Renderer.php

TODO:
1) cache management
2) don't expend the js on JsServlet.

> Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors
> ----------------------------------------------------------------------------------------------------
>
>                 Key: SHINDIG-1260
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-1260
>             Project: Shindig
>          Issue Type: Bug
>          Components: PHP
>    Affects Versions: 1.1-BETA5, 1.1-BETA6, 1.1-RC1
>         Environment: RedHat Linux 5.3, Apache 2.2.3, PHP 5.2.10
>            Reporter: James McIninch
>         Attachments: shindig-1260-jacky.patch, shindig-1260.patch
>
>
> The logic for selecting and inserting JavaScript using the GadgetFeatureRegistry and GadgetBaseRender is incorrect. The result is that JavaScript for features is included out-of-order of their dependencies, sometimes multiply included, and the 'gadgets' JavaScript object is not defined.
> Consider the following simple gadget:
> <?xml version="1.0" encoding="UTF-8"?>
> <Module>
> 	<ModulePrefs title="Hello" description="An example of a simple gadget" author="James McIninch" author_email="james.mcininc@biogenidec.com" />
> 	<UserPref name="firstname" display_name="Your name" datatype="string" default_value="stranger" />
> 	<Content type="html" preferred_height="50" preferred_width="200">
> 		<![CDATA[
> 		    <script type="text/javascript">
> 		       var prefs = new gadgets.Prefs();
> 		       var name  = prefs.getString("firstname");
> 		       document.getElementById("placeholder").innerHTML = name;
> 		    </script>
> 		    
> 		    Hello, <span id="placeholder">name goes here</span>!
> 		]]>
> 	</Content>
> </Module>
> This is rendered as:
> Hello, name goes here!
> Inspecting the JavaScript error console shows that 'gadgets' is undefined. However, the first instance of 'gadgets' being undefined lies in the Shindig-included JavaScript. This is because auth-init.js is included before config.js (both in features/src/main/javascript/features/core.*), but auth-init.js contains the line (28):
> shindig.auth = new shindig.Auth();
> which, in turn calls (auth.js line 159):
> gadgets.config.register("shindig.auth", null, init);
> ... but the 'gadgets' and 'gadgets.config' objects are not yet defined at that point. Despite being dependencies for the auth.js, they aren't  included in the order where the dependencies precede the code that depends on them.
> Also looking at the source of the rendered gadget, the taming.js code is included 5 separate times.
> Correcting the bug will entail re-thinking the logic of the feature ordering to include the dependency information and order the features based on the dependencies.
> All gadgets requiring the 'gadgets' object to be defined currently fail. Gadgets using the legacy model still function, though there exists redundant and erroneous JavaScript.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SHINDIG-1260) Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors

Posted by "Jacky Wang (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SHINDIG-1260?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12804439#action_12804439 ] 

Jacky Wang commented on SHINDIG-1260:
-------------------------------------

After some investigation, I found this problem might be a bit more complicated than it looks like.

1. I'm integrating James' features sorting code into GadgetFactory.
2. Where should we put the "forcedJsLib" logic in - GadgetFactory, or GadgetRendering part?  I suggest we putting it in GadgetFactory, thus GadgetRendering could render it straightforwardly.
3. The cache's behavior looks a bit strange - I'll merge them into GadgetRenderer.

The following are some notes:

GadgetRenderingServlet.php -> doGet
- create GadgetFactory
- create gadget by Factory

GadgetFactory.php -> createGadget
- fetch gadget content
- parse the content

GadgetSpecParser.php -> parseModulePrefs -> parseFeatures
1) specify the "required" features
2) specify the "optional" features
3) extract content rewrite settings (if exists)
4) add external/arbitary templates (if exists)
note that the features are not expanded yet - it's just an parsing process...

GadgetFactory.php -> parseFeatures
it is the real feature parsing process.
- call resolveFeatures, which calls addFeatureToResults recursively.
Therefore, the features are expanded in this step, and the features are categorized as "found" and "miss". The parsing flow could still go on if the missed feature is optional.

GadgetRenderingServlet.php -> renderGadget
It converts the gadget features (array of names) into the real "src=" or inline text.
Given the deriving hierarchy GadgetRenderer -> GadgetBaseRenderer -> [GadgetHtmlRenderer, GadgetUrlRenderer, GadgetHrefRenderer]:
- call Gadget***Renderer -> renderGadget



> Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors
> ----------------------------------------------------------------------------------------------------
>
>                 Key: SHINDIG-1260
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-1260
>             Project: Shindig
>          Issue Type: Bug
>          Components: PHP
>    Affects Versions: 1.1-BETA5, 1.1-BETA6, 1.1-RC1
>         Environment: RedHat Linux 5.3, Apache 2.2.3, PHP 5.2.10
>            Reporter: James McIninch
>         Attachments: shindig-1260.patch
>
>
> The logic for selecting and inserting JavaScript using the GadgetFeatureRegistry and GadgetBaseRender is incorrect. The result is that JavaScript for features is included out-of-order of their dependencies, sometimes multiply included, and the 'gadgets' JavaScript object is not defined.
> Consider the following simple gadget:
> <?xml version="1.0" encoding="UTF-8"?>
> <Module>
> 	<ModulePrefs title="Hello" description="An example of a simple gadget" author="James McIninch" author_email="james.mcininc@biogenidec.com" />
> 	<UserPref name="firstname" display_name="Your name" datatype="string" default_value="stranger" />
> 	<Content type="html" preferred_height="50" preferred_width="200">
> 		<![CDATA[
> 		    <script type="text/javascript">
> 		       var prefs = new gadgets.Prefs();
> 		       var name  = prefs.getString("firstname");
> 		       document.getElementById("placeholder").innerHTML = name;
> 		    </script>
> 		    
> 		    Hello, <span id="placeholder">name goes here</span>!
> 		]]>
> 	</Content>
> </Module>
> This is rendered as:
> Hello, name goes here!
> Inspecting the JavaScript error console shows that 'gadgets' is undefined. However, the first instance of 'gadgets' being undefined lies in the Shindig-included JavaScript. This is because auth-init.js is included before config.js (both in features/src/main/javascript/features/core.*), but auth-init.js contains the line (28):
> shindig.auth = new shindig.Auth();
> which, in turn calls (auth.js line 159):
> gadgets.config.register("shindig.auth", null, init);
> ... but the 'gadgets' and 'gadgets.config' objects are not yet defined at that point. Despite being dependencies for the auth.js, they aren't  included in the order where the dependencies precede the code that depends on them.
> Also looking at the source of the rendered gadget, the taming.js code is included 5 separate times.
> Correcting the bug will entail re-thinking the logic of the feature ordering to include the dependency information and order the features based on the dependencies.
> All gadgets requiring the 'gadgets' object to be defined currently fail. Gadgets using the legacy model still function, though there exists redundant and erroneous JavaScript.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SHINDIG-1260) Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors

Posted by "James McIninch (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SHINDIG-1260?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

James McIninch updated SHINDIG-1260:
------------------------------------

    Attachment: shindig-1260.patch

This patch fixes the reported issue.

> Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors
> ----------------------------------------------------------------------------------------------------
>
>                 Key: SHINDIG-1260
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-1260
>             Project: Shindig
>          Issue Type: Bug
>          Components: PHP
>    Affects Versions: 1.1-BETA5, 1.1-BETA6, 1.1-RC1
>         Environment: RedHat Linux 5.3, Apache 2.2.3, PHP 5.2.10
>            Reporter: James McIninch
>         Attachments: shindig-1260.patch
>
>
> The logic for selecting and inserting JavaScript using the GadgetFeatureRegistry and GadgetBaseRender is incorrect. The result is that JavaScript for features is included out-of-order of their dependencies, sometimes multiply included, and the 'gadgets' JavaScript object is not defined.
> Consider the following simple gadget:
> <?xml version="1.0" encoding="UTF-8"?>
> <Module>
> 	<ModulePrefs title="Hello" description="An example of a simple gadget" author="James McIninch" author_email="james.mcininc@biogenidec.com" />
> 	<UserPref name="firstname" display_name="Your name" datatype="string" default_value="stranger" />
> 	<Content type="html" preferred_height="50" preferred_width="200">
> 		<![CDATA[
> 		    <script type="text/javascript">
> 		       var prefs = new gadgets.Prefs();
> 		       var name  = prefs.getString("firstname");
> 		       document.getElementById("placeholder").innerHTML = name;
> 		    </script>
> 		    
> 		    Hello, <span id="placeholder">name goes here</span>!
> 		]]>
> 	</Content>
> </Module>
> This is rendered as:
> Hello, name goes here!
> Inspecting the JavaScript error console shows that 'gadgets' is undefined. However, the first instance of 'gadgets' being undefined lies in the Shindig-included JavaScript. This is because auth-init.js is included before config.js (both in features/src/main/javascript/features/core.*), but auth-init.js contains the line (28):
> shindig.auth = new shindig.Auth();
> which, in turn calls (auth.js line 159):
> gadgets.config.register("shindig.auth", null, init);
> ... but the 'gadgets' and 'gadgets.config' objects are not yet defined at that point. Despite being dependencies for the auth.js, they aren't  included in the order where the dependencies precede the code that depends on them.
> Also looking at the source of the rendered gadget, the taming.js code is included 5 separate times.
> Correcting the bug will entail re-thinking the logic of the feature ordering to include the dependency information and order the features based on the dependencies.
> All gadgets requiring the 'gadgets' object to be defined currently fail. Gadgets using the legacy model still function, though there exists redundant and erroneous JavaScript.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SHINDIG-1260) Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors

Posted by "Paul Lindner (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SHINDIG-1260?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12798895#action_12798895 ] 

Paul Lindner commented on SHINDIG-1260:
---------------------------------------

Can someone review this?  Once that's done I can commit it.

thanks


> Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors
> ----------------------------------------------------------------------------------------------------
>
>                 Key: SHINDIG-1260
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-1260
>             Project: Shindig
>          Issue Type: Bug
>          Components: PHP
>    Affects Versions: 1.1-BETA5, 1.1-BETA6, 1.1-RC1
>         Environment: RedHat Linux 5.3, Apache 2.2.3, PHP 5.2.10
>            Reporter: James McIninch
>         Attachments: shindig-1260.patch
>
>
> The logic for selecting and inserting JavaScript using the GadgetFeatureRegistry and GadgetBaseRender is incorrect. The result is that JavaScript for features is included out-of-order of their dependencies, sometimes multiply included, and the 'gadgets' JavaScript object is not defined.
> Consider the following simple gadget:
> <?xml version="1.0" encoding="UTF-8"?>
> <Module>
> 	<ModulePrefs title="Hello" description="An example of a simple gadget" author="James McIninch" author_email="james.mcininc@biogenidec.com" />
> 	<UserPref name="firstname" display_name="Your name" datatype="string" default_value="stranger" />
> 	<Content type="html" preferred_height="50" preferred_width="200">
> 		<![CDATA[
> 		    <script type="text/javascript">
> 		       var prefs = new gadgets.Prefs();
> 		       var name  = prefs.getString("firstname");
> 		       document.getElementById("placeholder").innerHTML = name;
> 		    </script>
> 		    
> 		    Hello, <span id="placeholder">name goes here</span>!
> 		]]>
> 	</Content>
> </Module>
> This is rendered as:
> Hello, name goes here!
> Inspecting the JavaScript error console shows that 'gadgets' is undefined. However, the first instance of 'gadgets' being undefined lies in the Shindig-included JavaScript. This is because auth-init.js is included before config.js (both in features/src/main/javascript/features/core.*), but auth-init.js contains the line (28):
> shindig.auth = new shindig.Auth();
> which, in turn calls (auth.js line 159):
> gadgets.config.register("shindig.auth", null, init);
> ... but the 'gadgets' and 'gadgets.config' objects are not yet defined at that point. Despite being dependencies for the auth.js, they aren't  included in the order where the dependencies precede the code that depends on them.
> Also looking at the source of the rendered gadget, the taming.js code is included 5 separate times.
> Correcting the bug will entail re-thinking the logic of the feature ordering to include the dependency information and order the features based on the dependencies.
> All gadgets requiring the 'gadgets' object to be defined currently fail. Gadgets using the legacy model still function, though there exists redundant and erroneous JavaScript.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SHINDIG-1260) Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors

Posted by "Chris Chabot (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SHINDIG-1260?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12799141#action_12799141 ] 

Chris Chabot commented on SHINDIG-1260:
---------------------------------------

I'll have this reviewed and committed by the end of the week, being in the middle of a relocation process slowed me down a bit more then usual :)

It's high on my list though since it seems to affect everyone and makes the trunk unstable

Initial review is good, but I need to run it through a few test cases before committing it

> Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors
> ----------------------------------------------------------------------------------------------------
>
>                 Key: SHINDIG-1260
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-1260
>             Project: Shindig
>          Issue Type: Bug
>          Components: PHP
>    Affects Versions: 1.1-BETA5, 1.1-BETA6, 1.1-RC1
>         Environment: RedHat Linux 5.3, Apache 2.2.3, PHP 5.2.10
>            Reporter: James McIninch
>         Attachments: shindig-1260.patch
>
>
> The logic for selecting and inserting JavaScript using the GadgetFeatureRegistry and GadgetBaseRender is incorrect. The result is that JavaScript for features is included out-of-order of their dependencies, sometimes multiply included, and the 'gadgets' JavaScript object is not defined.
> Consider the following simple gadget:
> <?xml version="1.0" encoding="UTF-8"?>
> <Module>
> 	<ModulePrefs title="Hello" description="An example of a simple gadget" author="James McIninch" author_email="james.mcininc@biogenidec.com" />
> 	<UserPref name="firstname" display_name="Your name" datatype="string" default_value="stranger" />
> 	<Content type="html" preferred_height="50" preferred_width="200">
> 		<![CDATA[
> 		    <script type="text/javascript">
> 		       var prefs = new gadgets.Prefs();
> 		       var name  = prefs.getString("firstname");
> 		       document.getElementById("placeholder").innerHTML = name;
> 		    </script>
> 		    
> 		    Hello, <span id="placeholder">name goes here</span>!
> 		]]>
> 	</Content>
> </Module>
> This is rendered as:
> Hello, name goes here!
> Inspecting the JavaScript error console shows that 'gadgets' is undefined. However, the first instance of 'gadgets' being undefined lies in the Shindig-included JavaScript. This is because auth-init.js is included before config.js (both in features/src/main/javascript/features/core.*), but auth-init.js contains the line (28):
> shindig.auth = new shindig.Auth();
> which, in turn calls (auth.js line 159):
> gadgets.config.register("shindig.auth", null, init);
> ... but the 'gadgets' and 'gadgets.config' objects are not yet defined at that point. Despite being dependencies for the auth.js, they aren't  included in the order where the dependencies precede the code that depends on them.
> Also looking at the source of the rendered gadget, the taming.js code is included 5 separate times.
> Correcting the bug will entail re-thinking the logic of the feature ordering to include the dependency information and order the features based on the dependencies.
> All gadgets requiring the 'gadgets' object to be defined currently fail. Gadgets using the legacy model still function, though there exists redundant and erroneous JavaScript.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (SHINDIG-1260) Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors

Posted by "Paul Lindner (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SHINDIG-1260?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Paul Lindner resolved SHINDIG-1260.
-----------------------------------

       Resolution: Fixed
    Fix Version/s: 1.1-BETA6

looks good.  patch applied.


> Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors
> ----------------------------------------------------------------------------------------------------
>
>                 Key: SHINDIG-1260
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-1260
>             Project: Shindig
>          Issue Type: Bug
>          Components: PHP
>    Affects Versions: 1.1-BETA5, 1.1-BETA6, 1.1-RC1
>         Environment: RedHat Linux 5.3, Apache 2.2.3, PHP 5.2.10
>            Reporter: James McIninch
>             Fix For: 1.1-BETA6
>
>         Attachments: shindig-1260-final.patch, shindig-1260.patch
>
>
> The logic for selecting and inserting JavaScript using the GadgetFeatureRegistry and GadgetBaseRender is incorrect. The result is that JavaScript for features is included out-of-order of their dependencies, sometimes multiply included, and the 'gadgets' JavaScript object is not defined.
> Consider the following simple gadget:
> <?xml version="1.0" encoding="UTF-8"?>
> <Module>
> 	<ModulePrefs title="Hello" description="An example of a simple gadget" author="James McIninch" author_email="james.mcininc@biogenidec.com" />
> 	<UserPref name="firstname" display_name="Your name" datatype="string" default_value="stranger" />
> 	<Content type="html" preferred_height="50" preferred_width="200">
> 		<![CDATA[
> 		    <script type="text/javascript">
> 		       var prefs = new gadgets.Prefs();
> 		       var name  = prefs.getString("firstname");
> 		       document.getElementById("placeholder").innerHTML = name;
> 		    </script>
> 		    
> 		    Hello, <span id="placeholder">name goes here</span>!
> 		]]>
> 	</Content>
> </Module>
> This is rendered as:
> Hello, name goes here!
> Inspecting the JavaScript error console shows that 'gadgets' is undefined. However, the first instance of 'gadgets' being undefined lies in the Shindig-included JavaScript. This is because auth-init.js is included before config.js (both in features/src/main/javascript/features/core.*), but auth-init.js contains the line (28):
> shindig.auth = new shindig.Auth();
> which, in turn calls (auth.js line 159):
> gadgets.config.register("shindig.auth", null, init);
> ... but the 'gadgets' and 'gadgets.config' objects are not yet defined at that point. Despite being dependencies for the auth.js, they aren't  included in the order where the dependencies precede the code that depends on them.
> Also looking at the source of the rendered gadget, the taming.js code is included 5 separate times.
> Correcting the bug will entail re-thinking the logic of the feature ordering to include the dependency information and order the features based on the dependencies.
> All gadgets requiring the 'gadgets' object to be defined currently fail. Gadgets using the legacy model still function, though there exists redundant and erroneous JavaScript.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SHINDIG-1260) Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors

Posted by "Jacky Wang (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SHINDIG-1260?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12803217#action_12803217 ] 

Jacky Wang commented on SHINDIG-1260:
-------------------------------------

Hi James, thank you for your effort on putting them together!

I've just did a quick review and here are some of the findings.  Please feel free to correct me.

1. the features are not constructed in the same routine:
  a) in GadgetFactory.php, the initial feature list is put together by combining both requiredFeatures and optionalFeatures.
  b) in GadgetBaseRenderer.php, where you modified, the forcedJsLibs are added and expended as well.
  c) in JsServlet.php, the features are fetched and expended again.  (this may introduce the multi-inclusion)

2. actually, the features are expended by GadgetFeatureRegistry->resolveFeatures, which calls GadgetFeatureRegistry->addFeatureToResults.

Your patch seems like to corrected the item 1.b with 2?

Thanks,
- Jacky

> Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors
> ----------------------------------------------------------------------------------------------------
>
>                 Key: SHINDIG-1260
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-1260
>             Project: Shindig
>          Issue Type: Bug
>          Components: PHP
>    Affects Versions: 1.1-BETA5, 1.1-BETA6, 1.1-RC1
>         Environment: RedHat Linux 5.3, Apache 2.2.3, PHP 5.2.10
>            Reporter: James McIninch
>         Attachments: shindig-1260.patch
>
>
> The logic for selecting and inserting JavaScript using the GadgetFeatureRegistry and GadgetBaseRender is incorrect. The result is that JavaScript for features is included out-of-order of their dependencies, sometimes multiply included, and the 'gadgets' JavaScript object is not defined.
> Consider the following simple gadget:
> <?xml version="1.0" encoding="UTF-8"?>
> <Module>
> 	<ModulePrefs title="Hello" description="An example of a simple gadget" author="James McIninch" author_email="james.mcininc@biogenidec.com" />
> 	<UserPref name="firstname" display_name="Your name" datatype="string" default_value="stranger" />
> 	<Content type="html" preferred_height="50" preferred_width="200">
> 		<![CDATA[
> 		    <script type="text/javascript">
> 		       var prefs = new gadgets.Prefs();
> 		       var name  = prefs.getString("firstname");
> 		       document.getElementById("placeholder").innerHTML = name;
> 		    </script>
> 		    
> 		    Hello, <span id="placeholder">name goes here</span>!
> 		]]>
> 	</Content>
> </Module>
> This is rendered as:
> Hello, name goes here!
> Inspecting the JavaScript error console shows that 'gadgets' is undefined. However, the first instance of 'gadgets' being undefined lies in the Shindig-included JavaScript. This is because auth-init.js is included before config.js (both in features/src/main/javascript/features/core.*), but auth-init.js contains the line (28):
> shindig.auth = new shindig.Auth();
> which, in turn calls (auth.js line 159):
> gadgets.config.register("shindig.auth", null, init);
> ... but the 'gadgets' and 'gadgets.config' objects are not yet defined at that point. Despite being dependencies for the auth.js, they aren't  included in the order where the dependencies precede the code that depends on them.
> Also looking at the source of the rendered gadget, the taming.js code is included 5 separate times.
> Correcting the bug will entail re-thinking the logic of the feature ordering to include the dependency information and order the features based on the dependencies.
> All gadgets requiring the 'gadgets' object to be defined currently fail. Gadgets using the legacy model still function, though there exists redundant and erroneous JavaScript.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.