On Wed, Aug 19, 2009 at 9:28 PM, Jasvir Nagra <[email protected]> wrote:
> > > 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. > Actually this change is still pending - http://codereview.appspot.com/104067/show. > > >> 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> >>> >>> >>> >> >
