[Caja] [google-caja commit] r3563 - fix 1056, NPE when cajoling , and relax about mandatory attrs</span></a></span> </h1> <p class="darkgray font13"> <span class="sender pipe"><a href="/search?l=google-caja-discuss@googlegroups.com&q=from:%22codesite%5C-noreply%22" rel="nofollow"><span itemprop="author" itemscope itemtype="http://schema.org/Person"><span itemprop="name">codesite-noreply</span></span></a></span> <span class="date"><a href="/search?l=google-caja-discuss@googlegroups.com&q=date:20090708" rel="nofollow">Wed, 08 Jul 2009 13:30:42 -0700</a></span> </p> </div> <div itemprop="articleBody" class="msgBody"> <!--X-Body-of-Message--> <pre style="margin: 0em;"> Author: felix8a Date: Wed Jul 8 13:28:33 2009 New Revision: 3563</pre><pre> Modified: trunk/src/com/google/caja/lang/html/html4-attributes-defs.json trunk/src/com/google/caja/plugin/templates/TemplateCompiler.java trunk/tests/com/google/caja/plugin/templates/TemplateCompilerTest.java Log: fix 1056, NPE when cajoling <textarea>, and relax about mandatory attrs <a rel="nofollow" href="http://codereview.appspot.com/90071">http://codereview.appspot.com/90071</a> The new html emitter is more aggressive about emitting default values for attributes that the html spec claims are not optional. 1. The spec for <textarea> says the cols= attribute isn't optional, but doesn't specify an acceptable default value. caja tries to emit a default anyway, which is an NPE. 2. It seems to me that caja should not be forcing default values if I haven't specified one. For example, <img> without alt= is meaningfully different from <img alt=''>. I don't see any reason for caja to normalize these attributes, rather than letting the browser do it naturally. r=ihab.a...@gmail.com Modified: trunk/src/com/google/caja/lang/html/html4-attributes-defs.json ============================================================================== </pre><tt>--- trunk/src/com/google/caja/lang/html/html4-attributes-defs.json </tt><tt>(original) </tt><tt>+++ trunk/src/com/google/caja/lang/html/html4-attributes-defs.json Wed Jul </tt><tt>8 13:28:33 2009 </tt><pre style="margin: 0em;"> @@ -5,7 +5,9 @@ </pre><tt> "Derived by taking </tt><tt><a rel="nofollow" href="http://www.w3.org/TR/html401/index/attributes.html"">http://www.w3.org/TR/html401/index/attributes.html"</a>;, </tt><tt> "and copying&pasting it into a text file which gave me a 7 column </tt><tt>file:", </tt><tt> " Name Related Elements Type Default Depr. DTD </tt><tt>Comment", </tt><pre style="margin: 0em;"> - "and running html4-attributes.pl." + "and running html4-attributes.pl.", </pre><tt>+ "Note, there are some deviations from html401's declaration of </tt><tt>optional", </tt><pre style="margin: 0em;"> + "because optional=false is used to force mandatory attributes." ], "types": [ @@ -90,9 +92,9 @@ { "key": "APPLET::ALT", "description": "short description", "optional": true }, { "key": "AREA::ALT", "description": "short description", - "optional": false }, + "optional": true }, { "key": "IMG::ALT", "description": "short description", - "optional": false }, + "optional": true }, { "key": "INPUT::ALT", "description": "short description", "optional": true }, </pre><tt> { "key": "APPLET::ARCHIVE", "description": "comma-separated archive </tt><tt>list", </tt><pre style="margin: 0em;"> @@ -194,7 +196,7 @@ </pre><tt> { "key": "FRAMESET::COLS", "description": "list of lengths, default: </tt><tt>100% (1 col)", </tt><pre style="margin: 0em;"> "optional": true }, { "key": "TEXTAREA::COLS", - "pattern": "[0-9]+", "optional": false }, + "pattern": "[0-9]+", "optional": true }, </pre><tt> { "key": "TD::COLSPAN", "description": "number of cols spanned by </tt><tt>cell", </tt><pre style="margin: 0em;"> "pattern": "[0-9]+", "default": "1", "optional": true }, </pre><tt> { "key": "TH::COLSPAN", "description": "number of cols spanned by </tt><tt>cell", </tt><pre style="margin: 0em;"> @@ -461,7 +463,7 @@ </pre><tt> { "key": "FRAMESET::ROWS", "description": "list of lengths, default: </tt><tt>100% (1 row)", </tt><pre style="margin: 0em;"> "optional": true }, { "key": "TEXTAREA::ROWS", - "pattern": "[0-9]+", "optional": false }, + "pattern": "[0-9]+", "optional": true }, </pre><tt> { "key": "TD::ROWSPAN", "description": "number of rows spanned by </tt><tt>cell", </tt><pre style="margin: 0em;"> "pattern": "[0-9]+", "default": "1", "optional": true }, </pre><tt> { "key": "TH::ROWSPAN", "description": "number of rows spanned by </tt><tt>cell", </tt><pre style="margin: 0em;"> @@ -507,7 +509,7 @@ { "key": "IFRAME::SRC", "description": "source of frame content", "mimeTypes": "text/html", "type": "URI", "optional": true }, { "key": "IMG::SRC", "description": "URI of image to embed", - "mimeTypes": "image/*", "type": "URI", "optional": false }, + "mimeTypes": "image/*", "type": "URI", "optional": true }, </pre><tt> { "key": "OBJECT::STANDBY", "description": "message to show while </tt><tt>loading", </tt><pre style="margin: 0em;"> "optional": true }, { "key": "OL::START", "description": "starting sequence number", @@ -553,9 +555,9 @@ </pre><tt> { "key": "PARAM::TYPE", "description": "content type for value when </tt><tt>valuetype=ref", </tt><pre style="margin: 0em;"> "optional": true }, </pre><tt> { "key": "SCRIPT::TYPE", "description": "content type of script </tt><tt>language", </tt><pre style="margin: 0em;"> - "optional": false }, + "optional": false, "default": "text/javascript" }, </pre><tt> { "key": "STYLE::TYPE", "description": "content type of style </tt><tt>language", </tt><pre style="margin: 0em;"> - "optional": false }, + "optional": false, "default": "text/css" }, </pre><tt> { "key": "INPUT::TYPE", "description": "what kind of widget is </tt><tt>needed", </tt><tt> "pattern": "TEXT|PASSWORD|CHECKBOX|RADIO|SUBMIT|RESET|FILE|HIDDEN| </tt><tt>IMAGE|BUTTON", "default": "TEXT", "optional": true }, </tt><pre style="margin: 0em;"> { "key": "LI::TYPE", "description": "list item style", Modified: trunk/src/com/google/caja/plugin/templates/TemplateCompiler.java ============================================================================== </pre><tt>--- trunk/src/com/google/caja/plugin/templates/TemplateCompiler.java </tt><tt>(original) </tt><tt>+++ trunk/src/com/google/caja/plugin/templates/TemplateCompiler.java Wed </tt><tt>Jul 8 13:28:33 2009 </tt><pre style="margin: 0em;"> @@ -228,6 +228,13 @@ } else { safeValue = a.getSafeValue(); } + if (safeValue == null) { + mq.addMessage(IhtmlMessageType.MISSING_ATTRIB, + Nodes.getFilePositionFor(el), + MessagePart.Factory.valueOf(elName.toString()), + MessagePart.Factory.valueOf(attrNameStr)); + continue; + } attr.setNodeValue(safeValue); el.setAttributeNode(attr); } </pre><tt>Modified: </tt><tt>trunk/tests/com/google/caja/plugin/templates/TemplateCompilerTest.java </tt><pre style="margin: 0em;"> ============================================================================== </pre><tt>--- trunk/tests/com/google/caja/plugin/templates/TemplateCompilerTest.java </tt><tt>(original) </tt><tt>+++ trunk/tests/com/google/caja/plugin/templates/TemplateCompilerTest.java </tt><tt>Wed Jul 8 13:28:33 2009 </tt><pre style="margin: 0em;"> @@ -245,7 +245,7 @@ assertSafeHtml( htmlFragment(fromString("<img src='blank.gif' width='20'/>")), htmlFragment(fromString( - "<img alt='' src='/proxy?url=test%3A%2Fblank.gif" + "<img src='/proxy?url=test%3A%2Fblank.gif" + "&amp;mime-type=image%2F%2A' width='20'/>")), new Block()); } @@ -336,6 +336,18 @@ + " emitter___.signalLoaded();" + "}" ))); + } + + /** + * <textarea> without cols= was triggering an NPE due to buggy handling + * of mandatory attributes. + * <a rel="nofollow" href="http://code.google.com/p/google-caja/issues/detail?id=1056">http://code.google.com/p/google-caja/issues/detail?id=1056</a> + */ + public void testBareTextarea() throws Exception { + assertSafeHtml( + htmlFragment(fromString("<textarea></textarea>")), + htmlFragment(fromString("<textarea></textarea>")), + new Block()); } private void assertSafeHtml( </pre> </div> <div class="msgButtons margintopdouble"> <ul class="overflow"> <li class="msgButtonItems"><a class="button buttonleft " accesskey="p" href="msg04104.html">Previous message</a></li> <li class="msgButtonItems textaligncenter"><a class="button" accesskey="c" href="index.html#04105">View by thread</a></li> <li class="msgButtonItems textaligncenter"><a class="button" accesskey="i" href="maillist.html#04105">View by date</a></li> <li class="msgButtonItems textalignright"><a class="button buttonright " accesskey="n" href="msg04107.html">Next message</a></li> </ul> </div> <a name="tslice"></a> <div class="tSliceList margintopdouble"> <ul class="icons monospace"> </ul> </div> <div class="overflow msgActions margintopdouble"> <div class="msgReply" > <h2> Reply via email to </h2> <form method="POST" action="/mailto.php"> <input type="hidden" name="subject" value="[Caja] [google-caja commit] r3563 - fix 1056, NPE when cajoling <textarea>, and relax about mandatory attrs"> <input type="hidden" name="msgid" value="000e0cd29bbc72bc57046e379cb2@google.com"> <input type="hidden" name="relpath" value="google-caja-discuss@googlegroups.com/msg04105.html"> <input type="submit" value=" codesite-noreply "> </form> </div> </div> </div> <div class="aside" role="complementary"> <div class="logo"> <a href="/"><img src="/logo.png" width=247 height=88 alt="The Mail Archive"></a> </div> <form class="overflow" action="/search" method="get"> <input type="hidden" name="l" value="google-caja-discuss@googlegroups.com"> <label class="hidden" for="q">Search the site</label> <input class="submittext" type="text" id="q" name="q" placeholder="Search google-caja-discuss"> <input class="submitbutton" name="submit" type="image" src="/submit.png" alt="Submit"> </form> <div class="nav margintop" id="nav" role="navigation"> <ul class="icons font16"> <li class="icons-home"><a href="/">The Mail Archive home</a></li> <li class="icons-list"><a href="/google-caja-discuss@googlegroups.com/">google-caja-discuss - all messages</a></li> <li class="icons-about"><a href="/google-caja-discuss@googlegroups.com/info.html">google-caja-discuss - about the list</a></li> <li class="icons-expand"><a href="/search?l=google-caja-discuss@googlegroups.com&q=subject:%22%5C%5BCaja%5C%5D+%5C%5Bgoogle%5C-caja+commit%5C%5D+r3563+%5C-+fix+1056%2C+NPE+when+cajoling+%3Ctextarea%3E%2C++and+relax+about+mandatory+attrs%22&o=newest&f=1" title="e" id="e">Expand</a></li> <li class="icons-prev"><a href="msg04104.html" title="p">Previous message</a></li> <li class="icons-next"><a href="msg04107.html" title="n">Next message</a></li> </ul> </div> <div class="listlogo margintopdouble"> </div> <div class="margintopdouble"> </div> </div> </div> <div class="footer" role="contentinfo"> <ul> <li><a href="/">The Mail Archive home</a></li> <li><a href="/faq.html#newlist">Add your mailing list</a></li> <li><a href="/faq.html">FAQ</a></li> <li><a href="/faq.html#support">Support</a></li> <li><a href="/faq.html#privacy">Privacy</a></li> <li class="darkgray">000e0cd29bbc72bc57046e379cb2@google.com</li> </ul> </div> </body> </html>