Better formating for static errors and warnings

* Displays errors in list if there was an error
* Displays warnings in a display:none div otherwise

@916053

[MikeSamuel, 2010-02-24 19:21:26]

I'm not sure I'm authorized to approve this, but LGTM with comments below.

http://codereview.appspot.com/217084/diff/1/2
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java
(right):

http://codereview.appspot.com/217084/diff/1/2#newcode157
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java:157:
messagesNode.setAttribute("style", "display: none");
Can this line move to the formatErrors, or can the node creation move here?

Setting an attribute clobbers anything that was there before, so it seems best
to have the stuff that sets up the DOM envelope in one place so that there's
less chance of someone setting a style there, and it being clobbered here.

http://codereview.appspot.com/217084/diff/1/2#newcode201
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java:201:
errElement.setAttribute("class", "gadgets-messages");
Maybe a comment as to where one would look for styles associated with this
class.

http://codereview.appspot.com/217084/diff/1/2#newcode210
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java:210:
.append(snippet);
Is there a reason the messageText is created outside this if, and a reason why
you don't do
   msg.getMessageLevel().name() + " " + html(...) + snippet
?
Do you need a break of some kind between the formatted message and the snippet?

Should the HtmlSnippetProducer be used at 198 instead?  It does have some
vestigial things, such as generating a call to an undefined function to scroll
to the source.  So maybe not.

http://codereview.appspot.com/217084/diff/1/3
File javascript/container/gadgets.css (right):

http://codereview.appspot.com/217084/diff/1/3#newcode63
javascript/container/gadgets.css:63: .gadgets-messages {
Maybe add a comment as to where this is generated.

[Jasvir, 2010-02-24 20:28:46]

http://codereview.appspot.com/217084/diff/1/2
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java
(right):

http://codereview.appspot.com/217084/diff/1/2#newcode157
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java:157:
messagesNode.setAttribute("style", "display: none");
On 2010/02/24 19:21:26, MikeSamuel wrote:
> Can this line move to the formatErrors, or can the node creation move here?
> 
> Setting an attribute clobbers anything that was there before, so it seems best
> to have the stuff that sets up the DOM envelope in one place so that there's
> less chance of someone setting a style there, and it being clobbered here.

Done.

http://codereview.appspot.com/217084/diff/1/2#newcode201
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java:201:
errElement.setAttribute("class", "gadgets-messages");
On 2010/02/24 19:21:26, MikeSamuel wrote:
> Maybe a comment as to where one would look for styles associated with this
> class.

Done.

http://codereview.appspot.com/217084/diff/1/2#newcode210
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/CajaContentRewriter.java:210:
.append(snippet);
On 2010/02/24 19:21:26, MikeSamuel wrote:
> Is there a reason the messageText is created outside this if, and a reason why
> you don't do
>    msg.getMessageLevel().name() + " " + html(...) + snippet
> ?
> Do you need a break of some kind between the formatted message and the
snippet?
> 
> Should the HtmlSnippetProducer be used at 198 instead?  It does have some
> vestigial things, such as generating a call to an undefined function to scroll
> to the source.  So maybe not.

Done.

I am not using HtmlSnippetProducer for exactly the reason you mention.

http://codereview.appspot.com/217084/diff/1/3
File javascript/container/gadgets.css (right):

http://codereview.appspot.com/217084/diff/1/3#newcode63
javascript/container/gadgets.css:63: .gadgets-messages {
On 2010/02/24 19:21:26, MikeSamuel wrote:
> Maybe add a comment as to where this is generated.

Done.

[MikeSamuel, 2010-02-24 20:38:33]

LGTM