2009/8/18 Kevin Brown <[email protected]> > It looks to me that at this point the only major barriers left to caja > adoption are taming related. I think we need: > 1. Tame osapi and various other libraries not covered by the big taming > blob currently in the caja feature >
The current taming was not designed well to handle the common pattern of callbacks. Notice the number of cases where functions essentially get rewritten to unwrap the cajoled code, call the original and rewrap to return a value to cajoled code. We're redoing the mechanism by which API developers tame their libraries - a library developer for each function will simply specify what the types of input parameters are and what the return value is - this will ease the load on the library developer. > 2. Refactor that big blob so that each feature does its own taming (the > caja feature shouldn't know anything about specific libraries) > All the mechanism for this is already there. The opensocialSchema object literal can be broken up and moved to individual features where the corresponding api is defined. Each feature can then register its taming with caja___.register() which are called when caja is loaded. As a convention, I suggest a taming.js file in each feature which is listed last in the corresponding feature.xml. > 3. Provide some clear instruction on the preferred way to deal with common > third party libraries. Are we going with a model where the entire DOM is > tamed and most stuff should just work, or will we need developers to use > tamed versions of each library (and somehow provide them). This is a very > confusing topic. > Do you mean libraries such as prototype, YUI and jQuery or feature libraries? If its the former, this should just work modulo some caching that will need to be done by Shindig - rather than cajoling the library each time. This is a serious performance bottleneck - addressing it makes an order of magnitude improvement rather than the linear improvements we get by addressing the reparsing issue. In an offline discussion some months ago, lryan suggested he would look at what it would take to address this. If its third-party feature libraries - library authors will need to provide a taming.js file which tames their API. As I mentioned earier, we're making good progress on a simpler way for API developers to specify how their library ought to be tamed. > > I think if we can address those 3 issues, most gadgets should be able to > work with valija. I'll be happy to take on 1 and 2 if caja folks could lend > some insight into #3. > Awesome. > > On Tue, Aug 18, 2009 at 11:36 AM, <[email protected]> wrote: > >> Author: lryan >> Date: Tue Aug 18 18:36:29 2009 >> New Revision: 805532 >> >> URL: http://svn.apache.org/viewvc?rev=805532&view=rev >> Log: >> Applied patch from Jasvir Nagra to avoid reparsing document while >> Cajoling. SHINDIG-1146. Includes update to Caja r3638 >> >> Modified: >> >> incubator/shindig/trunk/features/src/main/javascript/features/caja/taming.js >> >> >> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java >> incubator/shindig/trunk/pom.xml >> >> Modified: >> incubator/shindig/trunk/features/src/main/javascript/features/caja/taming.js >> URL: >> http://svn.apache.org/viewvc/incubator/shindig/trunk/features/src/main/javascript/features/caja/taming.js?rev=805532&r1=805531&r2=805532&view=diff >> >> ============================================================================== >> --- >> incubator/shindig/trunk/features/src/main/javascript/features/caja/taming.js >> (original) >> +++ >> incubator/shindig/trunk/features/src/main/javascript/features/caja/taming.js >> Tue Aug 18 18:36:29 2009 >> @@ -201,6 +201,8 @@ >> if (gadgets.views) >> gadgets.views.getCurrentView >> = taming.views.getCurrentView(gadgets.views.getCurrentView); >> + >> + if (opensocial) >> opensocial.newDataRequest = taming.newDataRequest(imports.$v, >> >> opensocial.newDataRequest); >> if (gadgets.MiniMessage) >> >> Modified: >> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java >> URL: >> http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java?rev=805532&r1=805531&r2=805532&view=diff >> >> ============================================================================== >> --- >> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java >> (original) >> +++ >> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java >> Tue Aug 18 18:36:29 2009 >> @@ -21,8 +21,10 @@ >> import org.apache.commons.lang.StringUtils; >> import org.apache.shindig.gadgets.Gadget; >> import org.apache.shindig.gadgets.rewrite.MutableContent; >> + >> import org.w3c.dom.Document; >> import org.w3c.dom.Element; >> +import org.w3c.dom.Node; >> >> import java.io.IOException; >> import java.io.InputStreamReader; >> @@ -32,16 +34,13 @@ >> import java.util.Map; >> import java.util.logging.Logger; >> >> -import com.google.caja.lexer.CharProducer; >> import com.google.caja.lexer.ExternalReference; >> -import com.google.caja.lexer.FilePosition; >> import com.google.caja.lexer.InputSource; >> import com.google.caja.lexer.escaping.Escaping; >> import com.google.caja.opensocial.DefaultGadgetRewriter; >> import com.google.caja.opensocial.GadgetRewriteException; >> import com.google.caja.opensocial.UriCallback; >> import com.google.caja.opensocial.UriCallbackException; >> -import com.google.caja.parser.html.Nodes; >> import com.google.caja.reporting.BuildInfo; >> import com.google.caja.reporting.Message; >> import com.google.caja.reporting.MessageContext; >> @@ -49,6 +48,7 @@ >> import com.google.caja.reporting.MessageQueue; >> import com.google.caja.reporting.SimpleMessageQueue; >> import com.google.caja.reporting.SnippetProducer; >> +import com.google.caja.util.Pair; >> import com.google.common.collect.Maps; >> >> public class CajaContentRewriter implements >> org.apache.shindig.gadgets.rewrite.GadgetRewriter { >> @@ -90,36 +90,52 @@ >> DefaultGadgetRewriter rw = new DefaultGadgetRewriter(bi, mq); >> rw.setValijaMode(true); >> InputSource is = new InputSource(retrievedUri); >> - String origContent = content.getContent(); >> - CharProducer input = CharProducer.Factory.create( >> - new StringReader(origContent), >> - FilePosition.instance(is, 5, 5, 5)); >> - StringBuilder output = new StringBuilder(); >> - >> Document doc = content.getDocument(); >> + Node root = doc.createDocumentFragment(); >> + root.appendChild(doc.getDocumentElement()); >> + boolean safe = false; >> try { >> - StringBuilder htmlAndJs = new StringBuilder(); >> - rw.rewriteContent(retrievedUri, input, cb, htmlAndJs); >> - int splitPoint = htmlAndJs.indexOf("<script"); >> - String script = htmlAndJs.substring(splitPoint); >> - String html = htmlAndJs.substring(0, splitPoint); >> - String htmlElement = >> - "<div id=\"cajoled-output\" class=\"g___\">" + >> - html + >> - "</div>"; >> - output.append(htmlElement); >> - output.append(tameCajaClientApi()); >> - output.append(script); >> - } catch (Exception e) { >> - content.setContent(messagesToHtml(doc, is, origContent, mq)); >> - throwCajolingException(e, mq); >> - return; >> + Pair<Node, Element> htmlAndJs = rw.rewriteContent(retrievedUri, >> root, >> + cb); >> + Node html = htmlAndJs.a; >> + Element script = htmlAndJs.b; >> + >> + Element cajoledOutput = doc.createElement("div"); >> + cajoledOutput.setAttribute("id", "cajoled-output"); >> + cajoledOutput.setAttribute("classes", "g___"); >> + cajoledOutput.appendChild(doc.adoptNode(html)); >> + cajoledOutput.appendChild(tameCajaClientApi(doc)); >> + cajoledOutput.appendChild(doc.adoptNode(script)); >> + >> + createContainerFor(doc, cajoledOutput); >> + content.documentChanged(); >> + safe = true; >> + } catch (GadgetRewriteException e) { >> + // There were cajoling errors >> + // Content is only used to produce useful snippets with error >> messages >> + createContainerFor(doc, formatErrors(doc, is, >> content.getContent(), mq)); >> + logException(e, mq); >> + } finally { >> + if (!safe) { >> + // Fail safe >> + content.setContent(""); >> + } >> } >> - content.setContent(output.toString()); >> } >> } >> >> - private String messagesToHtml(Document doc, InputSource is, >> CharSequence orig, MessageQueue mq) { >> + private void createContainerFor(Document doc, Element el) { >> + Element docEl = doc.createElement("html"); >> + Element head = doc.createElement("head"); >> + Element body = doc.createElement("body"); >> + doc.appendChild(docEl); >> + docEl.appendChild(head); >> + docEl.appendChild(body); >> + body.appendChild(el); >> + } >> + >> + private Element formatErrors(Document doc, InputSource is, >> + CharSequence orig, MessageQueue mq) { >> MessageContext mc = new MessageContext(); >> Map<InputSource, CharSequence> originalSrc = Maps.newHashMap(); >> originalSrc.put(is, orig); >> @@ -131,10 +147,10 @@ >> // Ignore LINT messages >> if (MessageLevel.LINT.compareTo(msg.getMessageLevel()) <= 0) { >> String snippet = sp.getSnippet(msg); >> - >> messageText.append(msg.getMessageLevel().name()) >> .append(" ") >> .append(html(msg.format(mc))); >> + >> if (!StringUtils.isEmpty(snippet)) { >> messageText.append("\n").append(snippet); >> } >> @@ -142,7 +158,7 @@ >> } >> Element errElement = doc.createElement("pre"); >> errElement.appendChild(doc.createTextNode(messageText.toString())); >> - return messageText.toString(); >> + return errElement; >> } >> >> private static String html(CharSequence s) { >> @@ -151,25 +167,22 @@ >> return sb.toString(); >> } >> >> - private String tameCajaClientApi() { >> - return "<script>___.enableCaja()</script>"; >> + private Element tameCajaClientApi(Document doc) { >> + Element scriptElement = doc.createElement("script"); >> + scriptElement.setAttribute("type", "text/javascript"); >> + scriptElement.appendChild(doc.createTextNode("___.enableCaja()")); >> + return scriptElement; >> } >> >> - private void throwCajolingException(Exception cause, MessageQueue mq) >> { >> + private void logException(Exception cause, MessageQueue mq) { >> StringBuilder errbuilder = new StringBuilder(); >> MessageContext mc = new MessageContext(); >> - >> if (cause != null) { >> errbuilder.append(cause).append('\n'); >> } >> - >> for (Message m : mq.getMessages()) { >> errbuilder.append(m.format(mc)).append('\n'); >> } >> - >> logger.info("Unable to cajole gadget: " + errbuilder); >> - >> - // throw new GadgetException( >> - // GadgetException.Code.MALFORMED_FOR_SAFE_INLINING, >> errbuilder.toString()); >> } >> } >> >> Modified: incubator/shindig/trunk/pom.xml >> URL: >> http://svn.apache.org/viewvc/incubator/shindig/trunk/pom.xml?rev=805532&r1=805531&r2=805532&view=diff >> >> ============================================================================== >> --- incubator/shindig/trunk/pom.xml (original) >> +++ incubator/shindig/trunk/pom.xml Tue Aug 18 18:36:29 2009 >> @@ -1247,7 +1247,7 @@ >> <dependency> >> <groupId>caja</groupId> >> <artifactId>caja</artifactId> >> - <version>r3574</version> >> + <version>r3638</version> >> <scope>compile</scope> >> </dependency> >> <dependency> >> >> >> >
