svn commit: r1004328 - in /shindig/trunk/java/gadgets/src: main/java/org/apache/shindig/gadgets/rewrite/ConcatVisitor.java test/java/org/apache/shindig/gadgets/rewrite/ConcatVisitorTest.java

[gagan]

Author: gagan
Date: Mon Oct  4 17:22:23 2010
New Revision: 1004328

URL: http://svn.apache.org/viewvc?rev=1004328&view=rev
Log:
Patch by satya3656 | Issue 2085042: Updated few checks in ConcatVisitor, to 
increase its coverage

Modified:
    
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatVisitor.java
    
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/ConcatVisitorTest.java

Modified: 
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatVisitor.java
URL: 
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatVisitor.java?rev=1004328&r1=1004327&r2=1004328&view=diff
==============================================================================
--- 
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatVisitor.java
 (original)
+++ 
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatVisitor.java
 Mon Oct  4 17:22:23 2010
@@ -247,9 +247,12 @@ public class ConcatVisitor implements Do
   private Element getSibling(Element root, boolean isPrev) {
     Node cur = root;
     while ((cur = getNext(cur, isPrev)) != null) {
-      if (cur.getNodeType() == Node.TEXT_NODE && 
StringUtils.isEmpty(cur.getTextContent())) {
+      // Text nodes are safe to skip, as they won't effect styles or scripts.
+      // It is also safe to skip comment nodes except for conditional comments.
+      if (cur.getNodeType() == Node.TEXT_NODE ||
+          (cur.getNodeType() == Node.COMMENT_NODE && 
!isConditionalComment(cur))) {
         continue;
-      }
+      } 
       break;
     }
     if (cur != null && cur.getNodeType() == Node.ELEMENT_NODE) {
@@ -302,4 +305,11 @@ public class ConcatVisitor implements Do
     // styles.
     return areLinkNodesCompatible && 
current.getAttribute("title").equals(next.getAttribute("title"));
   }
+
+  /**
+   * Checks if a given comment node is coditional comment.
+   */
+  private boolean isConditionalComment(Node node) {
+    return node.getNodeValue().trim().startsWith("[if");
+  }
 }

Modified: 
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/ConcatVisitorTest.java
URL: 
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/ConcatVisitorTest.java?rev=1004328&r1=1004327&r2=1004328&view=diff
==============================================================================
--- 
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/ConcatVisitorTest.java
 (original)
+++ 
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/ConcatVisitorTest.java
 Mon Oct  4 17:22:23 2010
@@ -187,10 +187,26 @@ public class ConcatVisitorTest extends D
   }
 
   @Test
-  public void dontVisitCssSeperatedByNonEmptyTextNode() throws Exception {
+  public void visitCssSeperatedByTextNode() throws Exception {
     Node textNode = doc.createTextNode("Data\n");
     Node node = elem("link", "type", "text/css", "rel", "stylesheet", "href", 
CSS1_URL_STR);
     seqNodes(node, textNode, css1);
+    assertEquals(VisitStatus.RESERVE_NODE, getVisitStatusCss(node, null));
+  }
+
+  @Test
+  public void visitCssSeperatedByNormalComment() throws Exception {
+    Node commentNode = doc.createComment("This is a comment");
+    Node node = elem("link", "type", "text/css", "rel", "stylesheet", "href", 
CSS1_URL_STR);
+    seqNodes(node, commentNode, css1);
+    assertEquals(VisitStatus.RESERVE_NODE, getVisitStatusCss(node, null));
+  }
+
+  @Test
+  public void dontVisitCssSeperatedByConditionalComment() throws Exception {
+    Node commentNode = doc.createComment("[if IE]");
+    Node node = elem("link", "type", "text/css", "rel", "stylesheet", "href", 
CSS1_URL_STR);
+    seqNodes(node, commentNode, css1);
     assertEquals(VisitStatus.BYPASS, getVisitStatusCss(node, null));
   }