From 705d1c84c9a03bb0378593e4d8d8a90c8a11f831 Mon Sep 17 00:00:00 2001
From: gmungoc <g.m.carstairs@dundee.ac.uk>
Date: Thu, 26 Oct 2017 11:56:27 +0100
Subject: [PATCH] JAL-2792 html table for Feature Details report

---
 src/jalview/datamodel/SequenceFeature.java      |   47 ++++++++++++----------
 src/jalview/gui/CutAndPasteHtmlTransfer.java    |    1 +
 src/jalview/gui/PopupMenu.java                  |    8 +++-
 src/jalview/io/SequenceAnnotationReport.java    |   48 +++--------------------
 src/jalview/jbgui/GCutAndPasteHtmlTransfer.java |   19 +++++++++
 src/jalview/util/StringUtils.java               |   41 +++++++++++++++++++
 test/jalview/datamodel/SequenceFeatureTest.java |   43 ++++++++++++++++++++
 test/jalview/util/StringUtilsTest.java          |   22 +++++++++++
 8 files changed, 165 insertions(+), 64 deletions(-)

diff --git a/src/jalview/datamodel/SequenceFeature.java b/src/jalview/datamodel/SequenceFeature.java
index 4208ce1..f5a9b42 100755
--- a/src/jalview/datamodel/SequenceFeature.java
+++ b/src/jalview/datamodel/SequenceFeature.java
@@ -21,6 +21,7 @@
 package jalview.datamodel;
 
 import jalview.datamodel.features.FeatureLocationI;
+import jalview.util.StringUtils;
 
 import java.util.HashMap;
 import java.util.Map;
@@ -29,10 +30,9 @@ import java.util.TreeMap;
 import java.util.Vector;
 
 /**
- * DOCUMENT ME!
- * 
- * @author $author$
- * @version $Revision$
+ * A class that models a single contiguous feature on a sequence. If flag
+ * 'contactFeature' is true, the start and end positions are interpreted instead
+ * as two contact points.
  */
 public class SequenceFeature implements FeatureLocationI
 {
@@ -52,6 +52,8 @@ public class SequenceFeature implements FeatureLocationI
   // private key for ENA location designed not to conflict with real GFF data
   private static final String LOCATION = "!Location";
 
+  private static final String ROW_DATA = "<tr><td>%s</td><td>%s</td></tr>";
+
   /*
    * map of otherDetails special keys, and their value fields' delimiter
    */
@@ -548,30 +550,28 @@ public class SequenceFeature implements FeatureLocationI
   }
 
   /**
-   * Answers a formatted text report of feature details
+   * Answers an html-formatted report of feature details
    * 
    * @return
    */
   public String getDetailsReport()
   {
     StringBuilder sb = new StringBuilder(128);
-    if (begin == end)
-    {
-      sb.append(String.format("%s %d %s", type, begin, description));
-    }
-    else
-    {
-      sb.append(String.format("%s %d-%d %s", type, begin, end, description));
-    }
+    sb.append("<br>");
+    sb.append("<table>");
+    sb.append(String.format(ROW_DATA, "Type", type));
+    sb.append(String.format(ROW_DATA, "Start/end", begin == end ? begin
+            : begin + (isContactFeature() ? ":" : "-") + end));
+    String desc = StringUtils.stripHtmlTags(description);
+    sb.append(String.format(ROW_DATA, "Description", desc));
     if (!Float.isNaN(score) && score != 0f)
     {
-      sb.append(" score=").append(score);
+      sb.append(String.format(ROW_DATA, "Score", score));
     }
     if (featureGroup != null)
     {
-      sb.append(" (").append(featureGroup).append(")");
+      sb.append(String.format(ROW_DATA, "Group", featureGroup));
     }
-    sb.append("\n\n");
 
     if (otherDetails != null)
     {
@@ -586,22 +586,29 @@ public class SequenceFeature implements FeatureLocationI
         {
           continue; // to avoid double reporting
         }
+        sb.append("<tr><td>").append(key).append("</td><td>");
         if (INFO_KEYS.containsKey(key))
         {
           /*
            * split selected INFO data by delimiter over multiple lines
            */
-          sb.append(key).append("=\n  ");
+          sb.append("</td></tr>");
           String delimiter = INFO_KEYS.get(key);
-          String value = entry.getValue().toString();
-          sb.append(value.replace(delimiter, "\n  "));
+          String[] values = entry.getValue().toString().split(delimiter);
+          for (String value : values)
+          {
+            sb.append("<tr><td>&nbsp</td><td>").append(value)
+                    .append("</td></tr>");
+          }
         }
         else
         {
-          sb.append(key + "=" + entry.getValue().toString() + "\n");
+          sb.append(entry.getValue().toString()).append("</td></tr>");
         }
       }
     }
+    sb.append("</table>");
+
     String text = sb.toString();
     return text;
   }
diff --git a/src/jalview/gui/CutAndPasteHtmlTransfer.java b/src/jalview/gui/CutAndPasteHtmlTransfer.java
index 71a1520..2e51bce 100644
--- a/src/jalview/gui/CutAndPasteHtmlTransfer.java
+++ b/src/jalview/gui/CutAndPasteHtmlTransfer.java
@@ -141,6 +141,7 @@ public class CutAndPasteHtmlTransfer extends GCutAndPasteHtmlTransfer
    */
   public void setText(String text)
   {
+    textarea.setDocument(textarea.getEditorKit().createDefaultDocument());
     textarea.setText(text);
   }
 
diff --git a/src/jalview/gui/PopupMenu.java b/src/jalview/gui/PopupMenu.java
index 40f5764..6da7d4f 100644
--- a/src/jalview/gui/PopupMenu.java
+++ b/src/jalview/gui/PopupMenu.java
@@ -49,6 +49,7 @@ import jalview.schemes.PIDColourScheme;
 import jalview.util.GroupUrlLink;
 import jalview.util.GroupUrlLink.UrlStringTooLongException;
 import jalview.util.MessageManager;
+import jalview.util.StringUtils;
 import jalview.util.UrlLink;
 
 import java.awt.Color;
@@ -552,6 +553,7 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener
       String description = sf.getDescription();
       if (description != null)
       {
+        description = StringUtils.stripHtmlTags(description);
         if (description.length() <= 6)
         {
           desc = desc + " " + description;
@@ -585,8 +587,12 @@ public class PopupMenu extends JPopupMenu implements ColourChangeListener
    */
   protected void showFeatureDetails(SequenceFeature sf)
   {
-    CutAndPasteTransfer cap = new CutAndPasteTransfer();
+    CutAndPasteHtmlTransfer cap = new CutAndPasteHtmlTransfer();
+    // it appears Java's CSS does not support border-collaps :-(
+    cap.addStylesheetRule("table { border-collapse: collapse;}");
+    cap.addStylesheetRule("table, td, th {border: 1px solid black;}");
     cap.setText(sf.getDetailsReport());
+
     Desktop.addInternalFrame(cap,
             MessageManager.getString("label.feature_details"), 500, 500);
   }
diff --git a/src/jalview/io/SequenceAnnotationReport.java b/src/jalview/io/SequenceAnnotationReport.java
index 13f41d4..6d819d3 100644
--- a/src/jalview/io/SequenceAnnotationReport.java
+++ b/src/jalview/io/SequenceAnnotationReport.java
@@ -26,6 +26,7 @@ import jalview.datamodel.SequenceFeature;
 import jalview.datamodel.SequenceI;
 import jalview.io.gff.GffConstants;
 import jalview.util.MessageManager;
+import jalview.util.StringUtils;
 import jalview.util.UrlLink;
 
 import java.util.Arrays;
@@ -183,50 +184,11 @@ public class SequenceAnnotationReport
           sb.append(" ").append(feature.end);
         }
 
-        if (feature.getDescription() != null
-                && !feature.description.equals(feature.getType()))
+        String description = feature.getDescription();
+        if (description != null && !description.equals(feature.getType()))
         {
-          String tmpString = feature.getDescription();
-          String tmp2up = tmpString.toUpperCase();
-          int startTag = tmp2up.indexOf("<HTML>");
-          if (startTag > -1)
-          {
-            tmpString = tmpString.substring(startTag + 6);
-            tmp2up = tmp2up.substring(startTag + 6);
-          }
-          int endTag = tmp2up.indexOf("</BODY>");
-          if (endTag > -1)
-          {
-            tmpString = tmpString.substring(0, endTag);
-            tmp2up = tmp2up.substring(0, endTag);
-          }
-          endTag = tmp2up.indexOf("</HTML>");
-          if (endTag > -1)
-          {
-            tmpString = tmpString.substring(0, endTag);
-          }
-
-          if (startTag > -1)
-          {
-            sb.append("; ").append(tmpString);
-          }
-          else
-          {
-            if (tmpString.indexOf("<") > -1 || tmpString.indexOf(">") > -1)
-            {
-              // The description does not specify html is to
-              // be used, so we must remove < > symbols
-              tmpString = tmpString.replaceAll("<", "&lt;");
-              tmpString = tmpString.replaceAll(">", "&gt;");
-
-              sb.append("; ");
-              sb.append(tmpString);
-            }
-            else
-            {
-              sb.append("; ").append(tmpString);
-            }
-          }
+          description = StringUtils.stripHtmlTags(description);
+          sb.append("; ").append(description);
         }
         // check score should be shown
         if (!Float.isNaN(feature.getScore()))
diff --git a/src/jalview/jbgui/GCutAndPasteHtmlTransfer.java b/src/jalview/jbgui/GCutAndPasteHtmlTransfer.java
index abc0b3d..a6e0ace 100644
--- a/src/jalview/jbgui/GCutAndPasteHtmlTransfer.java
+++ b/src/jalview/jbgui/GCutAndPasteHtmlTransfer.java
@@ -39,6 +39,8 @@ import javax.swing.JMenuBar;
 import javax.swing.JMenuItem;
 import javax.swing.JPanel;
 import javax.swing.JScrollPane;
+import javax.swing.text.EditorKit;
+import javax.swing.text.html.HTMLEditorKit;
 
 /**
  * DOCUMENT ME!
@@ -85,6 +87,7 @@ public class GCutAndPasteHtmlTransfer extends JInternalFrame
   {
     try
     {
+      textarea.setEditorKit(new HTMLEditorKit());
       setJMenuBar(editMenubar);
       jbInit();
     } catch (Exception e)
@@ -272,4 +275,20 @@ public class GCutAndPasteHtmlTransfer extends JInternalFrame
   {
 
   }
+
+  /**
+   * Adds the given stylesheet rule to the Html editor. However note that CSS
+   * support is limited.
+   * 
+   * @param rule
+   * @see javax.swing.text.html.CSS
+   */
+  public void addStylesheetRule(String rule)
+  {
+    EditorKit editorKit = textarea.getEditorKit();
+    if (editorKit != null)
+    {
+      ((HTMLEditorKit) editorKit).getStyleSheet().addRule(rule);
+    }
+  }
 }
diff --git a/src/jalview/util/StringUtils.java b/src/jalview/util/StringUtils.java
index b3456aa..2e8ace8 100644
--- a/src/jalview/util/StringUtils.java
+++ b/src/jalview/util/StringUtils.java
@@ -403,4 +403,45 @@ public class StringUtils
     }
     return s.substring(0, 1).toUpperCase() + s.substring(1).toLowerCase();
   }
+
+  /**
+   * A helper method that strips off any leading or trailing html and body tags.
+   * If no html tag is found, then also html-encodes angle bracket characters.
+   * 
+   * @param text
+   * @return
+   */
+  public static String stripHtmlTags(String text)
+  {
+    if (text == null)
+    {
+      return null;
+    }
+    String tmp2up = text.toUpperCase();
+    int startTag = tmp2up.indexOf("<HTML>");
+    if (startTag > -1)
+    {
+      text = text.substring(startTag + 6);
+      tmp2up = tmp2up.substring(startTag + 6);
+    }
+    // is omission of "<BODY>" intentional here??
+    int endTag = tmp2up.indexOf("</BODY>");
+    if (endTag > -1)
+    {
+      text = text.substring(0, endTag);
+      tmp2up = tmp2up.substring(0, endTag);
+    }
+    endTag = tmp2up.indexOf("</HTML>");
+    if (endTag > -1)
+    {
+      text = text.substring(0, endTag);
+    }
+  
+    if (startTag == -1 && (text.contains("<") || text.contains(">")))
+    {
+      text = text.replaceAll("<", "&lt;");
+      text = text.replaceAll(">", "&gt;");
+    }
+    return text;
+  }
 }
diff --git a/test/jalview/datamodel/SequenceFeatureTest.java b/test/jalview/datamodel/SequenceFeatureTest.java
index fbeb365..8c9cbc9 100644
--- a/test/jalview/datamodel/SequenceFeatureTest.java
+++ b/test/jalview/datamodel/SequenceFeatureTest.java
@@ -273,4 +273,47 @@ public class SequenceFeatureTest
             "group");
     assertTrue(sf.isContactFeature());
   }
+
+  @Test(groups = { "Functional" })
+  public void testGetDetailsReport()
+  {
+    // single locus, no group, no score
+    SequenceFeature sf = new SequenceFeature("variant", "G,C", 22, 22, null);
+    String expected = "<br><table><tr><td>Type</td><td>variant</td></tr>"
+            + "<tr><td>Start/end</td><td>22</td></tr>"
+            + "<tr><td>Description</td><td>G,C</td></tr></table>";
+    assertEquals(expected, sf.getDetailsReport());
+
+    // contact feature
+    sf = new SequenceFeature("Disulphide Bond", "a description", 28, 31,
+            null);
+    expected = "<br><table><tr><td>Type</td><td>Disulphide Bond</td></tr>"
+            + "<tr><td>Start/end</td><td>28:31</td></tr>"
+            + "<tr><td>Description</td><td>a description</td></tr></table>";
+    assertEquals(expected, sf.getDetailsReport());
+
+    sf = new SequenceFeature("variant", "G,C", 22, 33,
+            12.5f, "group");
+    sf.setValue("Parent", "ENSG001");
+    sf.setValue("Child", "ENSP002");
+    expected = "<br><table><tr><td>Type</td><td>variant</td></tr>"
+            + "<tr><td>Start/end</td><td>22-33</td></tr>"
+            + "<tr><td>Description</td><td>G,C</td></tr>"
+            + "<tr><td>Score</td><td>12.5</td></tr>"
+            + "<tr><td>Group</td><td>group</td></tr>"
+            + "<tr><td>Child</td><td>ENSP002</td></tr>"
+            + "<tr><td>Parent</td><td>ENSG001</td></tr></table>";
+    assertEquals(expected, sf.getDetailsReport());
+
+    /*
+     * feature with embedded html link in description
+     */
+    String desc = "<html>Fer2 Status: True Positive <a href=\"http://pfam.xfam.org/family/PF00111\">Pfam 8_8</a></html>";
+    sf = new SequenceFeature("Pfam", desc, 8, 83, "Uniprot");
+    expected = "<br><table><tr><td>Type</td><td>Pfam</td></tr>"
+            + "<tr><td>Start/end</td><td>8-83</td></tr>"
+            + "<tr><td>Description</td><td>Fer2 Status: True Positive <a href=\"http://pfam.xfam.org/family/PF00111\">Pfam 8_8</a></td></tr>"
+            + "<tr><td>Group</td><td>Uniprot</td></tr></table>";
+    assertEquals(expected, sf.getDetailsReport());
+  }
 }
diff --git a/test/jalview/util/StringUtilsTest.java b/test/jalview/util/StringUtilsTest.java
index b6f8a25..084219a 100644
--- a/test/jalview/util/StringUtilsTest.java
+++ b/test/jalview/util/StringUtilsTest.java
@@ -228,4 +228,26 @@ public class StringUtilsTest
     assertEquals("", StringUtils.toSentenceCase(""));
     assertNull(StringUtils.toSentenceCase(null));
   }
+
+  @Test(groups = { "Functional" })
+  public void testStripHtmlTags()
+  {
+    assertNull(StringUtils.stripHtmlTags(null));
+    assertEquals("", StringUtils.stripHtmlTags(""));
+    assertEquals(
+            "<a href=\"something\">label</href>",
+            StringUtils
+                    .stripHtmlTags("<html><a href=\"something\">label</href></html>"));
+
+    // if no "<html>" tag, < and > get html-encoded (not sure why)
+    assertEquals("&lt;a href=\"something\"&gt;label&lt;/href&gt;",
+            StringUtils.stripHtmlTags("<a href=\"something\">label</href>"));
+
+    // </body> gets removed but not <body> (is this intentional?)
+    assertEquals("<body><p>hello",
+            StringUtils.stripHtmlTags("<html><body><p>hello</body></html>"));
+
+    assertEquals("kdHydro &lt; 12.53",
+            StringUtils.stripHtmlTags("kdHydro < 12.53"));
+  }
 }
-- 
1.7.10.2