You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by "Bruno Bowden (JIRA)" <ji...@apache.org> on 2008/02/21 19:03:19 UTC

[jira] Created: (SHINDIG-88) "quirks" attribute, multiple view refactoring

"quirks" attribute, multiple view refactoring
---------------------------------------------

                 Key: SHINDIG-88
                 URL: https://issues.apache.org/jira/browse/SHINDIG-88
             Project: Shindig
          Issue Type: Improvement
          Components: Gadgets Server - Java
            Reporter: Bruno Bowden
            Assignee: Kevin Brown


- <Content quirks="false"> allows developer to request standards mode
   <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">  
- Moves view information in to separate class
- Adds extensive test cases for multiple views, content / moduleprefs attributes and parse errors

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SHINDIG-88) "quirks" attribute, multiple view refactoring

Posted by "Paul Lindner (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SHINDIG-88?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12571235#action_12571235 ] 

Paul Lindner commented on SHINDIG-88:
-------------------------------------

I'd love to see this integrated!

One less patch to maintain


> "quirks" attribute, multiple view refactoring
> ---------------------------------------------
>
>                 Key: SHINDIG-88
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-88
>             Project: Shindig
>          Issue Type: Improvement
>          Components: Gadgets Server - Java
>            Reporter: Bruno Bowden
>            Assignee: Kevin Brown
>         Attachments: quirks-begone.patch
>
>
> - <Content quirks="false"> allows developer to request standards mode
>    <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">  
> - Moves view information in to separate class
> - Adds extensive test cases for multiple views, content / moduleprefs attributes and parse errors

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Closed: (SHINDIG-88) "quirks" attribute, multiple view refactoring

Posted by "Kevin Brown (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SHINDIG-88?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kevin Brown closed SHINDIG-88.
------------------------------

    Resolution: Fixed

This has been applied.

> "quirks" attribute, multiple view refactoring
> ---------------------------------------------
>
>                 Key: SHINDIG-88
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-88
>             Project: Shindig
>          Issue Type: Improvement
>          Components: Gadgets Server - Java
>            Reporter: Bruno Bowden
>            Assignee: Kevin Brown
>         Attachments: quirks-begone.patch, quirks-begone2.patch
>
>
> - <Content quirks="false"> allows developer to request standards mode
>    <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">  
> - Moves view information in to separate class
> - Adds extensive test cases for multiple views, content / moduleprefs attributes and parse errors

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SHINDIG-88) "quirks" attribute, multiple view refactoring

Posted by "Kevin Brown (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SHINDIG-88?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12571411#action_12571411 ] 

Kevin Brown commented on SHINDIG-88:
------------------------------------

Index: java/gadgets/src/main/java/org/apache/shindig/gadgets/http/CrossServletState.java
===================================================================
--- java/gadgets/src/main/java/org/apache/shindig/gadgets/http/CrossServletState.java	(revision 629861)
+++ java/gadgets/src/main/java/org/apache/shindig/gadgets/http/CrossServletState.java	(working copy)
@@ -101,7 +101,7 @@
    * context object). A better choice would probably be to add the view params
    * to ProcessingOptions and pass that here.
    */
-  public abstract String getIframeUrl(Gadget gadget,  ProcessingOptions opts);
+  public abstract String getIframeUrl(Gadget gadget, String viewName, ProcessingOptions opts);

I don't think this signature should be changed -- instead, the view should be obtained from ProcessingOptions (as would anything that comes from a url parameter).  It might actually make more sense to remove this method altogether since only RpcServlet uses it.
 

Index: java/gadgets/src/main/java/org/apache/shindig/gadgets/http/GadgetRenderingServlet.java
===================================================================
--- java/gadgets/src/main/java/org/apache/shindig/gadgets/http/GadgetRenderingServlet.java	(revision 629861)
+++ java/gadgets/src/main/java/org/apache/shindig/gadgets/http/GadgetRenderingServlet.java	(working copy)
-    markup.append("</head><body>");
+    markup.append(
+        "<html>\n" +
+        "  <head>\n" +
+        "    <style type=\"text/css\">\n" +
+        "      body,td,div,span,p{font-family:arial,sans-serif;}\n" +
+        "      a {color:#0000cc;}a:visited {color:#551a8b;}\n" +
+        "      a:active {color:#ff0000;}\n" +
+        "      body{margin: 0px;padding: 0px;background-color:white;}\n" +
+        "    </style>\n" +
+        "  </head>\n" +
+        "  <body>\n");

Leave off the newlines here. They don't make any difference in the java code, and the html output is unreadable anyway.  Eventually all markup will be minified anyway, so this situation will only get worse.

     StringBuilder externJs = new StringBuilder();
     StringBuilder inlineJs = new StringBuilder();
-    String externFmt = "<script src=\"%s\"></script>\n";
+    String externFmt =
+        "    <script src=\"%s\"></script>\n";

Leave this out as well. It actually makes the java harder to read.

 
     markup.append(content);
-    markup.append("<script>gadgets.util.runOnLoadHandlers();</script>");
-    markup.append("</body></html>");
+    markup.append(
+        "    <script>gadgets.util.runOnLoadHandlers();</script>\n");
+    markup.append(
+        "  </body>\n" +
+        "</html>");

And these.
 
Index: java/gadgets/src/main/java/org/apache/shindig/gadgets/Gadget.java
===================================================================
--- java/gadgets/src/main/java/org/apache/shindig/gadgets/Gadget.java	(revision 629861)
+++ java/gadgets/src/main/java/org/apache/shindig/gadgets/Gadget.java	(working copy)
@@ -17,6 +17,8 @@
-  public ContentType getContentType() {
-    return baseSpec.getContentType();
+  public String debugString() {
+    return baseSpec.debugString();

toString is supposed to serve this purpose.

Index: java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecParser.java
===================================================================
--- java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecParser.java	(revision 629861)
+++ java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecParser.java	(working copy)
+    for (int i = 0; i < attrs.getLength(); i++) {
+      Node attr = attrs.item(i);
+      if (!MODULEPREFS_KNOWN_ATTRS_SET.contains(attr.getNodeName())) {
+        // TODO: in debug mode, this should warn developer about unknown attributes
+      }
+    }

This belongs in a static block, but I don't really think it's the right way to deal with warning developers about unknown attributes -- we should pass the gadget through the XSD validator and look for errors produced in there instead. This way we can maintain a single XSD for everything that shindig supports (most likely a superset of the extended spec). 
 
+    NamedNodeMap attrsNodeMap = content.getAttributes();
+    String contentData = content.getTextContent();
+    Map<String, String> attributes = new HashMap<String, String>();
+    for (int idx = 0; idx < attrsNodeMap.getLength(); idx++) {
+      Node attr = attrsNodeMap.item(idx);
+      String attrName = attr.getNodeName();
+      if (attrName == "view")
+        continue;
+      attributes.put(attrName, attr.getNodeValue());
     }

Copy attributes here is wasteful -- NamedNodeMap is already a map (though it doesn't implement the Java map interface). If you want to pass a map of attributes to addContent, just pass the NamedNodeMap here.

+          if (href == null)

Brackets

+      private static final String quirksAttrName = "quirks";
+      private static final String typeAttrName = "type";
+      private static final String hrefAttrName = "href";

constants should be ALL_CAPS.

+        if (attrs == null)
+          attrs = attributes;
+        if (attrs.containsKey(name) == false)
+          return defaultValue;
+        String value = attrs.get(name);
+        if (value.equals("true"))
+          return true;
+        if (value.equals("false"))
+          return false;

brackets

Index: java/gadgets/src/main/java/org/apache/shindig/gadgets/SpecParserException.java
===================================================================
--- java/gadgets/src/main/java/org/apache/shindig/gadgets/SpecParserException.java	(revision 629861)
+++ java/gadgets/src/main/java/org/apache/shindig/gadgets/SpecParserException.java	(working copy)
@@ -28,7 +28,7 @@
     super(GadgetException.Code.MALFORMED_XML_DOCUMENT, message);
   }
 
-  public SpecParserException(GadgetException.Code code) {
-    super(code);
+  public SpecParserException(GadgetException.Code code, String message) {
+    super(code, message);

Make this a new method and leave the existing ctor as is. 

> "quirks" attribute, multiple view refactoring
> ---------------------------------------------
>
>                 Key: SHINDIG-88
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-88
>             Project: Shindig
>          Issue Type: Improvement
>          Components: Gadgets Server - Java
>            Reporter: Bruno Bowden
>            Assignee: Kevin Brown
>         Attachments: quirks-begone.patch
>
>
> - <Content quirks="false"> allows developer to request standards mode
>    <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">  
> - Moves view information in to separate class
> - Adds extensive test cases for multiple views, content / moduleprefs attributes and parse errors

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SHINDIG-88) "quirks" attribute, multiple view refactoring

Posted by "Bruno Bowden (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SHINDIG-88?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Bruno Bowden updated SHINDIG-88:
--------------------------------

    Attachment: quirks-begone.patch

> "quirks" attribute, multiple view refactoring
> ---------------------------------------------
>
>                 Key: SHINDIG-88
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-88
>             Project: Shindig
>          Issue Type: Improvement
>          Components: Gadgets Server - Java
>            Reporter: Bruno Bowden
>            Assignee: Kevin Brown
>         Attachments: quirks-begone.patch
>
>
> - <Content quirks="false"> allows developer to request standards mode
>    <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">  
> - Moves view information in to separate class
> - Adds extensive test cases for multiple views, content / moduleprefs attributes and parse errors

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SHINDIG-88) "quirks" attribute, multiple view refactoring

Posted by "Bruno Bowden (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SHINDIG-88?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Bruno Bowden updated SHINDIG-88:
--------------------------------

    Attachment: quirks-begone2.patch

Code reviewed and improved.

- Url parameter for view
- Improved html indentation for debug mode
- Added debugging information for unspecified or invalid view.

This example swaps between quirks and standards mode depending on whether view "a" or view "b" is used. 

"a" View - Quirks Mode
http://bruno-corp.corp.google.com:8080/gadgets/ifr?url=http://hosting.gmodules.com/ig/gadgets/file/115843805466700406260/ref-multiple-views.xml&view=a&debug&nocache

"b" View - Standards Mode
http://bruno-corp.corp.google.com:8080/gadgets/ifr?url=http://hosting.gmodules.com/ig/gadgets/file/115843805466700406260/ref-multiple-views.xml&view=b&debug&nocache

> "quirks" attribute, multiple view refactoring
> ---------------------------------------------
>
>                 Key: SHINDIG-88
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-88
>             Project: Shindig
>          Issue Type: Improvement
>          Components: Gadgets Server - Java
>            Reporter: Bruno Bowden
>            Assignee: Kevin Brown
>         Attachments: quirks-begone.patch, quirks-begone2.patch
>
>
> - <Content quirks="false"> allows developer to request standards mode
>    <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">  
> - Moves view information in to separate class
> - Adds extensive test cases for multiple views, content / moduleprefs attributes and parse errors

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SHINDIG-88) "quirks" attribute, multiple view refactoring

Posted by "Kevin Brown (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SHINDIG-88?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12572634#action_12572634 ] 

Kevin Brown commented on SHINDIG-88:
------------------------------------

You probably don't want to attach corp urls to this issue either.

On Tue, Feb 26, 2008 at 11:52 AM, Bruno Bowden (JIRA) <ji...@apache.org>



-- 
~Kevin

If you received this email by mistake, please delete it, cancel your mail
account, destroy your hard drive, silence any witnesses, and burn down the
building that you're in.


> "quirks" attribute, multiple view refactoring
> ---------------------------------------------
>
>                 Key: SHINDIG-88
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-88
>             Project: Shindig
>          Issue Type: Improvement
>          Components: Gadgets Server - Java
>            Reporter: Bruno Bowden
>            Assignee: Kevin Brown
>         Attachments: quirks-begone.patch, quirks-begone2.patch
>
>
> - <Content quirks="false"> allows developer to request standards mode
>    <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">  
> - Moves view information in to separate class
> - Adds extensive test cases for multiple views, content / moduleprefs attributes and parse errors

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SHINDIG-88) "quirks" attribute, multiple view refactoring

Posted by "Bruno Bowden (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SHINDIG-88?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12571249#action_12571249 ] 

Bruno Bowden commented on SHINDIG-88:
-------------------------------------

Public description of issue on spec discussion list:
http://groups.google.com/group/opensocial-and-gadgets-spec/browse_thread/thread/fa8e8863f91b6333/5889b3fe50629966#5889b3fe50629966

> "quirks" attribute, multiple view refactoring
> ---------------------------------------------
>
>                 Key: SHINDIG-88
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-88
>             Project: Shindig
>          Issue Type: Improvement
>          Components: Gadgets Server - Java
>            Reporter: Bruno Bowden
>            Assignee: Kevin Brown
>         Attachments: quirks-begone.patch
>
>
> - <Content quirks="false"> allows developer to request standards mode
>    <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">  
> - Moves view information in to separate class
> - Adds extensive test cases for multiple views, content / moduleprefs attributes and parse errors

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SHINDIG-88) "quirks" attribute, multiple view refactoring

Posted by "Bruno Bowden (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SHINDIG-88?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12574166#action_12574166 ] 

Bruno Bowden commented on SHINDIG-88:
-------------------------------------

Email gadgets-support@google.com for support questions and
gadgets-eng@google.com for the team.


> "quirks" attribute, multiple view refactoring
> ---------------------------------------------
>
>                 Key: SHINDIG-88
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-88
>             Project: Shindig
>          Issue Type: Improvement
>          Components: Gadgets Server - Java
>            Reporter: Bruno Bowden
>            Assignee: Kevin Brown
>         Attachments: quirks-begone.patch, quirks-begone2.patch
>
>
> - <Content quirks="false"> allows developer to request standards mode
>    <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">  
> - Moves view information in to separate class
> - Adds extensive test cases for multiple views, content / moduleprefs attributes and parse errors

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.