shindig-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jacky Wang (JIRA)" <>
Subject [jira] Updated: (SHINDIG-1260) Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript errors
Date Mon, 01 Feb 2010 13:11:50 GMT


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:

The code-review could be found here:

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.


> Improper JavaScript inclusion logic leads to out-of-order and duplicate includes / JavaScript
> ----------------------------------------------------------------------------------------------------
>                 Key: SHINDIG-1260
>                 URL:
>             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="" />
> 	<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
> 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.

View raw message