Author: etnu
Date: Sun Nov 23 19:10:28 2008
New Revision: 720104

URL: http://svn.apache.org/viewvc?rev=720104&view=rev
Log:
Fixed behavior that was lost during the transition to document based rewriting:

- Ensured that default CSS is only injected when no doctype is present. This 
default CSS only exists to preserve legacy compatibility with old gadgets 
authored for igoogle, and is not necessary for any other gadgets.

Also simplified injection rules to avoid the need to extract all elements from 
the base document.


Modified:
    
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingContentRewriter.java
    
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/RenderingContentRewriterTest.java

Modified: 
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingContentRewriter.java
URL: 
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingContentRewriter.java?rev=720104&r1=720103&r2=720104&view=diff
==============================================================================
--- 
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingContentRewriter.java
 (original)
+++ 
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingContentRewriter.java
 Sun Nov 23 19:10:28 2008
@@ -47,21 +47,19 @@
 import org.apache.shindig.gadgets.spec.UserPref;
 import org.apache.shindig.gadgets.spec.View;
 
-import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
 import com.google.inject.Inject;
 
 import org.json.JSONException;
 import org.json.JSONObject;
+import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 import org.w3c.dom.Node;
-import org.w3c.dom.NodeList;
 import org.w3c.dom.Text;
 
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.Iterator;
-import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.logging.Level;
@@ -86,7 +84,7 @@
 public class RenderingContentRewriter implements ContentRewriter {
   private static final Logger LOG = 
Logger.getLogger(RenderingContentRewriter.class.getName());
 
-  static final String DEFAULT_HEAD_CONTENT =
+  static final String DEFAULT_CSS =
       "body,td,div,span,p{font-family:arial,sans-serif;}" +
       "a {color:#0000cc;}a:visited {color:#551a8b;}" +
       "a:active {color:#ff0000;}" +
@@ -120,38 +118,33 @@
 
   public RewriterResults rewrite(Gadget gadget, MutableContent mutableContent) 
{
     try {
-      Element head = (Element)DomUtil.getFirstNamedChildNode(
-          mutableContent.getDocument().getDocumentElement(), "head");
+      Document document = mutableContent.getDocument();
 
-      // Remove all the elements currently in head and add them back after we 
inject content
-      List<Node> existingHeadContent = Lists.newArrayList();
-      NodeList children = head.getChildNodes();
-      for (int i = 0; i < children.getLength(); i++) {
-        existingHeadContent.add(children.item(i));
-      }
-      for (Node n : existingHeadContent) {
-        head.removeChild(n);
-      }
-
-      Element defaultStyle = head.getOwnerDocument().createElement("style");
-      defaultStyle.setAttribute("type", "text/css");
-      head.appendChild(defaultStyle);
-      defaultStyle.appendChild(defaultStyle.getOwnerDocument().
-          createTextNode(DEFAULT_HEAD_CONTENT));
+      Element head = 
(Element)DomUtil.getFirstNamedChildNode(document.getDocumentElement(), "head");
+
+      // Only inject default styles if no doctype was specified.
+      if (document.getDoctype() == null) {
+        Element defaultStyle = document.createElement("style");
+        defaultStyle.setAttribute("type", "text/css");
+        head.appendChild(defaultStyle);
+        defaultStyle.appendChild(defaultStyle.getOwnerDocument().
+            createTextNode(DEFAULT_CSS));
+      }
 
       injectBaseTag(gadget, head);
       injectFeatureLibraries(gadget, head);
 
       // This can be one script block.
-      Element mainScriptTag = head.getOwnerDocument().createElement("script");
+      Element mainScriptTag = document.createElement("script");
       injectMessageBundles(gadget, mainScriptTag);
       injectDefaultPrefs(gadget, mainScriptTag);
       injectPreloads(gadget, mainScriptTag);
-      head.appendChild(mainScriptTag);
 
-      Element body = (Element)DomUtil.getFirstNamedChildNode(
-          mutableContent.getDocument().getDocumentElement(), "body");
-      
+      // We need to inject our script before any developer scripts.
+      head.insertBefore(mainScriptTag, DomUtil.getFirstNamedChildNode(head, 
"script"));
+
+      Element body = 
(Element)DomUtil.getFirstNamedChildNode(document.getDocumentElement(), "body");
+
       LocaleSpec localeSpec = gadget.getLocale();
       if (localeSpec != null) {
         body.setAttribute("dir", localeSpec.getLanguageDirection());
@@ -159,11 +152,7 @@
 
       injectOnLoadHandlers(body);
 
-      for (Node n : existingHeadContent) {
-        head.appendChild(n);
-      }
-
-      MutableContent.notifyEdit(mutableContent.getDocument());
+      mutableContent.documentChanged();
       return RewriterResults.notCacheable();
     } catch (GadgetException e) {
       // TODO: Rewriter interface needs to be modified to handle 
GadgetException or
@@ -182,7 +171,7 @@
       }
       Element baseTag = headTag.getOwnerDocument().createElement("base");
       baseTag.setAttribute("href", base.toString());
-      headTag.appendChild(baseTag);
+      headTag.insertBefore(baseTag, headTag.getFirstChild());
     }
   }
 

Modified: 
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/RenderingContentRewriterTest.java
URL: 
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/RenderingContentRewriterTest.java?rev=720104&r1=720103&r2=720104&view=diff
==============================================================================
--- 
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/RenderingContentRewriterTest.java
 (original)
+++ 
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/RenderingContentRewriterTest.java
 Sun Nov 23 19:10:28 2008
@@ -18,6 +18,14 @@
  */
 package org.apache.shindig.gadgets.render;
 
+import static 
org.apache.shindig.gadgets.render.RenderingContentRewriter.DEFAULT_CSS;
+import static 
org.apache.shindig.gadgets.render.RenderingContentRewriter.FEATURES_KEY;
+import static 
org.apache.shindig.gadgets.render.RenderingContentRewriter.INSERT_BASE_ELEMENT_KEY;
+import static org.easymock.EasyMock.expect;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
 import org.apache.shindig.common.ContainerConfig;
 import org.apache.shindig.common.PropertiesModule;
 import org.apache.shindig.common.uri.Uri;
@@ -36,9 +44,6 @@
 import org.apache.shindig.gadgets.preload.PreloadException;
 import org.apache.shindig.gadgets.preload.PreloadedData;
 import org.apache.shindig.gadgets.preload.Preloads;
-import static 
org.apache.shindig.gadgets.render.RenderingContentRewriter.DEFAULT_HEAD_CONTENT;
-import static 
org.apache.shindig.gadgets.render.RenderingContentRewriter.FEATURES_KEY;
-import static 
org.apache.shindig.gadgets.render.RenderingContentRewriter.INSERT_BASE_ELEMENT_KEY;
 import org.apache.shindig.gadgets.rewrite.MutableContent;
 import org.apache.shindig.gadgets.spec.GadgetSpec;
 import org.apache.shindig.gadgets.spec.LocaleSpec;
@@ -52,14 +57,10 @@
 import com.google.inject.Guice;
 import com.google.inject.Injector;
 
-import static org.easymock.EasyMock.expect;
 import org.easymock.classextension.EasyMock;
 import org.easymock.classextension.IMocksControl;
 import org.json.JSONException;
 import org.json.JSONObject;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -137,8 +138,7 @@
     assertTrue("Output is not valid HTML.", matcher.matches());
     assertTrue("Missing opening html tag", matcher.group(BEFORE_HEAD_GROUP).
         toLowerCase().contains("<html>"));
-    assertTrue("Default head content is missing.",
-        matcher.group(HEAD_GROUP).contains(DEFAULT_HEAD_CONTENT));
+    assertTrue("Default CSS missing.", 
matcher.group(HEAD_GROUP).contains(DEFAULT_CSS));
     // Not very accurate -- could have just been user prefs.
     assertTrue("Default javascript not included.",
         matcher.group(HEAD_GROUP).contains("<script>"));
@@ -188,6 +188,8 @@
     assertTrue("Custom head content is missing.", 
matcher.group(HEAD_GROUP).contains(head));
     assertTrue("Forced javascript not included.",
         matcher.group(HEAD_GROUP).contains("<script src=\"/js/foo\">"));
+    assertFalse("Default styling was injected when a doctype was specified.",
+        matcher.group(HEAD_GROUP).contains(DEFAULT_CSS));
     assertTrue("Custom body attributes missing.",
         matcher.group(BODY_ATTRIBUTES_GROUP).contains(bodyAttr));
     assertTrue("Original document not preserved.",
@@ -637,9 +639,10 @@
     Matcher matcher = DOCUMENT_SPLIT_PATTERN.matcher(content);
     assertTrue("Output is not valid HTML.", matcher.matches());
     Pattern baseElementPattern
-        = Pattern.compile("(?:.*)<base href=\"(.*?)\">(?:.*)", Pattern.DOTALL);
+        = Pattern.compile("^<base href=\"(.*?)\">(?:.*)", Pattern.DOTALL);
     Matcher baseElementMatcher = 
baseElementPattern.matcher(matcher.group(HEAD_GROUP));
-    assertTrue("base element missing from head of document.", 
baseElementMatcher.matches());
+    assertTrue("Base element does not exist at the beginning of the head 
element.",
+        baseElementMatcher.matches());
     return baseElementMatcher.group(1);
   }
 


Reply via email to