Move the style related nodes to top of the head node instead of appending at the end.

[Optimization]
Move the style related nodes to top of the head node instead of appending at the end in StyleAdjacencyVisitor.

Keeping the link tags before script tags will help in parallelize the resource fetching as the script fetching is blocking.

[satya3656, 2010-08-25 09:32:08]

Uploading Patch

[gagan.goku, 2010-08-25 09:35:44]

http://codereview.appspot.com/2013046/diff/2001/3002
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleAdjacencyVisitor.java
(right):

http://codereview.appspot.com/2013046/diff/2001/3002#newcode71
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleAdjacencyVisitor.java:71:
head.insertBefore(currentNode, firstChild);
wont this insert the nodes in reverse order ?

[satya3656, 2010-08-25 09:54:01]

http://codereview.appspot.com/2013046/diff/2001/3002
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleAdjacencyVisitor.java
(right):

http://codereview.appspot.com/2013046/diff/2001/3002#newcode71
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleAdjacencyVisitor.java:71:
head.insertBefore(currentNode, firstChild);
On 2010/08/25 09:35:44, gagan.goku wrote:
> wont this insert the nodes in reverse order ?

FirstChild() of the head in the previous dom.
Does this name firstChildOfOriginalHead sound better?.

[Kuntal Loya, 2010-08-25 12:28:38]

http://codereview.appspot.com/2013046/diff/2001/3002
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleAdjacencyVisitor.java
(right):

http://codereview.appspot.com/2013046/diff/2001/3002#newcode44
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleAdjacencyVisitor.java:44:
("stylesheet".equalsIgnoreCase(getAttrib(node, "rel")) ||
Should this also be && ?

http://codereview.appspot.com/2013046/diff/2001/3002#newcode46
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleAdjacencyVisitor.java:46:
// Reserve <style...>, <link rel="stylesheet"...>, or <link type="*css*"...>
Comment needs to be updated.

http://codereview.appspot.com/2013046/diff/2001/3002#newcode67
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleAdjacencyVisitor.java:67:
Node currentNode = node.getParentNode().removeChild(node);
What if firstChild is same as currentNode ?

Could you please add a test for the same.

http://codereview.appspot.com/2013046/diff/2001/3001
File
java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/StyleAdjacencyVisitorTest.java
(right):

http://codereview.appspot.com/2013046/diff/2001/3001#newcode119
java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/StyleAdjacencyVisitorTest.java:119:
assertSame(style, head.getChildNodes().item(0)); // Last.
// First.

http://codereview.appspot.com/2013046/diff/2001/3001#newcode138
java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/StyleAdjacencyVisitorTest.java:138:
assertSame(style, head.getChildNodes().item(0)); // Last.
// First.

[satya3656, 2010-08-25 12:47:49]

http://codereview.appspot.com/2013046/diff/2001/3002
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleAdjacencyVisitor.java
(right):

http://codereview.appspot.com/2013046/diff/2001/3002#newcode44
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleAdjacencyVisitor.java:44:
("stylesheet".equalsIgnoreCase(getAttrib(node, "rel")) ||
On 2010/08/25 12:28:38, Kuntal Loya wrote:
> Should this also be && ?

we want to even pull the atlernate style sheets, as it may lead to reorder.

http://codereview.appspot.com/2013046/diff/2001/3002#newcode46
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleAdjacencyVisitor.java:46:
// Reserve <style...>, <link rel="stylesheet"...>, or <link type="*css*"...>
On 2010/08/25 12:28:38, Kuntal Loya wrote:
> Comment needs to be updated.

Done.

http://codereview.appspot.com/2013046/diff/2001/3002#newcode67
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/StyleAdjacencyVisitor.java:67:
Node currentNode = node.getParentNode().removeChild(node);
On 2010/08/25 12:28:38, Kuntal Loya wrote:
> What if firstChild is same as currentNode ?
> 
> Could you please add a test for the same.

Good find, logic will get screwed, changing the logic.

http://codereview.appspot.com/2013046/diff/2001/3001
File
java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/StyleAdjacencyVisitorTest.java
(right):

http://codereview.appspot.com/2013046/diff/2001/3001#newcode119
java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/StyleAdjacencyVisitorTest.java:119:
assertSame(style, head.getChildNodes().item(0)); // Last.
On 2010/08/25 12:28:38, Kuntal Loya wrote:
> // First.

Done.

http://codereview.appspot.com/2013046/diff/2001/3001#newcode138
java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/StyleAdjacencyVisitorTest.java:138:
assertSame(style, head.getChildNodes().item(0)); // Last.
On 2010/08/25 12:28:38, Kuntal Loya wrote:
> // First.

Done.

[satya3656, 2010-08-25 12:48:17]

Addressing the comments

[gagan.goku, 2010-08-25 18:14:26]

lgtm. You want might to sync this patch to head now that
http://codereview.appspot.com/1997041/ has been submitted.

[satya3656, 2010-08-30 12:02:18]

Sync to head(r990766)

[Paul Lindner, 2010-08-30 20:25:03]

committed.

[gagan.goku, 2010-08-31 02:58:25]

Thanks Paul.

-- 
The only thing missing in life is background music.
-- Gagandeep Singh


On Tue, Aug 31, 2010 at 1:55 AM, <lindner@inuus.com> wrote:

> committed.
>
>
>
> http://codereview.appspot.com/2013046/
>

[satya3656, 2010-08-31 07:07:42]

Thanks Paul

On 31 August 2010 01:55, <lindner@inuus.com> wrote:

> committed.
>
>
>
> http://codereview.appspot.com/2013046/
>