Merge branch 'develop' into features/JAL-1723_sequenceReport
authorgmungoc <g.m.carstairs@dundee.ac.uk>
Mon, 31 Oct 2016 10:25:06 +0000 (10:25 +0000)
committergmungoc <g.m.carstairs@dundee.ac.uk>
Mon, 31 Oct 2016 10:25:06 +0000 (10:25 +0000)
Conflicts:
src/jalview/io/SequenceAnnotationReport.java

1  2 
src/jalview/appletgui/APopupMenu.java
src/jalview/gui/IdPanel.java
src/jalview/gui/PopupMenu.java
src/jalview/gui/SeqPanel.java
src/jalview/io/SequenceAnnotationReport.java
test/jalview/io/SequenceAnnotationReportTest.java

@@@ -43,7 -43,6 +43,6 @@@ import jalview.schemes.HelixColourSchem
  import jalview.schemes.HydrophobicColourScheme;
  import jalview.schemes.NucleotideColourScheme;
  import jalview.schemes.PIDColourScheme;
- import jalview.schemes.ResidueProperties;
  import jalview.schemes.StrandColourScheme;
  import jalview.schemes.TaylorColourScheme;
  import jalview.schemes.TurnColourScheme;
@@@ -70,8 -69,6 +69,6 @@@ import java.util.Vector
  public class APopupMenu extends java.awt.PopupMenu implements
          ActionListener, ItemListener
  {
-   private static final String ALL_ANNOTATIONS = "All";
    Menu groupMenu = new Menu();
  
    MenuItem editGroupName = new MenuItem();
  
      CutAndPasteTransfer cap = new CutAndPasteTransfer(false, ap.alignFrame);
  
 -    StringBuffer contents = new StringBuffer();
 +    StringBuilder contents = new StringBuilder(128);
      for (SequenceI seq : sequences)
      {
        contents.append(MessageManager.formatMessage(
                seq,
                true,
                true,
 -              false,
                (ap.seqPanel.seqCanvas.fr != null) ? ap.seqPanel.seqCanvas.fr
                        .getMinMax() : null);
        contents.append("</p>");
              "label.represent_group_with", new Object[] { "" }));
      revealAll.setLabel(MessageManager.getString("action.reveal_all"));
      revealSeq.setLabel(MessageManager.getString("action.reveal_sequences"));
-     menu1.setLabel(MessageManager.getString("label.group") + ":");
+     menu1.setLabel(MessageManager.getString("label.group:"));
      add(groupMenu);
      this.add(seqMenu);
      this.add(hideSeqs);
      if (conservationMenuItem.getState())
      {
  
-       sg.cs.setConservation(Conservation.calculateConservation("Group",
-               ResidueProperties.propHash, 3, sg.getSequences(ap.av
-                       .getHiddenRepSequences()), 0, ap.av.getAlignment()
-                       .getWidth(), false, ap.av.getConsPercGaps(), false));
+       sg.cs.setConservation(Conservation.calculateConservation("Group", 3,
+               sg.getSequences(ap.av.getHiddenRepSequences()), 0, ap.av
+                       .getAlignment().getWidth(), false, ap.av
+                       .getConsPercGaps(), false));
        SliderPanel.setConservationSlider(ap, sg.cs, sg.getName());
        SliderPanel.showConservationSlider();
      }
  
    void hideSequences(boolean representGroup)
    {
-     SequenceGroup sg = ap.av.getSelectionGroup();
-     if (sg == null || sg.getSize() < 1)
-     {
-       ap.av.hideSequence(new SequenceI[] { seq });
-       return;
-     }
-     ap.av.setSelectionGroup(null);
-     if (representGroup)
-     {
-       ap.av.hideRepSequences(seq, sg);
-       return;
-     }
-     int gsize = sg.getSize();
-     SequenceI[] hseqs;
-     hseqs = new SequenceI[gsize];
-     int index = 0;
-     for (int i = 0; i < gsize; i++)
-     {
-       hseqs[index++] = sg.getSequenceAt(i);
-     }
-     ap.av.hideSequence(hseqs);
-     ap.av.sendSelection();
+     ap.av.hideSequences(seq, representGroup);
    }
  
    /**
      showMenu.removeAll();
      hideMenu.removeAll();
  
-     final List<String> all = Arrays.asList(ALL_ANNOTATIONS);
+     final List<String> all = Arrays.asList(new String[] { MessageManager
+             .getString("label.all") });
      addAnnotationTypeToShowHide(showMenu, forSequences, "", all, true, true);
      addAnnotationTypeToShowHide(hideMenu, forSequences, "", all, true,
              false);
@@@ -26,6 -26,7 +26,7 @@@ import jalview.datamodel.SequenceGroup
  import jalview.datamodel.SequenceI;
  import jalview.io.SequenceAnnotationReport;
  import jalview.util.MessageManager;
+ import jalview.util.Platform;
  import jalview.util.UrlLink;
  import jalview.viewmodel.AlignmentViewport;
  
@@@ -108,8 -109,8 +109,8 @@@ public class IdPanel extends JPanel imp
      if (seq > -1 && seq < av.getAlignment().getHeight())
      {
        SequenceI sequence = av.getAlignment().getSequenceAt(seq);
 -      StringBuffer tip = new StringBuffer(64);
 -      seqAnnotReport.createSequenceAnnotationReport(tip, sequence,
 +      StringBuilder tip = new StringBuilder(64);
 +      seqAnnotReport.createTooltipAnnotationReport(tip, sequence,
                av.isShowDBRefs(), av.isShowNPFeats(),
                sp.seqCanvas.fr.getMinMax());
        setToolTipText(JvSwingUtils.wrapTooltip(true,
       */
      if (e.getClickCount() < 2 || SwingUtilities.isRightMouseButton(e))
      {
+       // reinstate isRightMouseButton check to ignore mouse-related popup events
+       // note - this does nothing on default MacBookPro force-trackpad config!
        return;
      }
  
        return;
      }
  
-     int seq = alignPanel.getSeqPanel().findSeq(e);
-     if (SwingUtilities.isRightMouseButton(e))
+     if (e.isPopupTrigger()) // Mac reports this in mousePressed
      {
-       Sequence sq = (Sequence) av.getAlignment().getSequenceAt(seq);
-       // build a new links menu based on the current links + any non-positional
-       // features
-       Vector nlinks = new Vector(Preferences.sequenceURLLinks);
-       SequenceFeature sf[] = sq == null ? null : sq.getSequenceFeatures();
-       for (int sl = 0; sf != null && sl < sf.length; sl++)
-       {
-         if (sf[sl].begin == sf[sl].end && sf[sl].begin == 0)
-         {
-           if (sf[sl].links != null && sf[sl].links.size() > 0)
-           {
-             for (int l = 0, lSize = sf[sl].links.size(); l < lSize; l++)
-             {
-               nlinks.addElement(sf[sl].links.elementAt(l));
-             }
-           }
-         }
-       }
-       jalview.gui.PopupMenu pop = new jalview.gui.PopupMenu(alignPanel, sq,
-               nlinks, new Vector(Preferences.getGroupURLLinks()));
-       pop.show(this, e.getX(), e.getY());
+       showPopupMenu(e);
+       return;
+     }
  
+     /*
+      * defer right-mouse click handling to mouseReleased on Windows
+      * (where isPopupTrigger() will answer true)
+      * NB isRightMouseButton is also true for Cmd-click on Mac
+      */
+     if (SwingUtilities.isRightMouseButton(e) && !Platform.isAMac())
+     {
        return;
      }
  
      if ((av.getSelectionGroup() == null)
-             || ((!e.isControlDown() && !e.isShiftDown()) && av
+             || (!jalview.util.Platform.isControlDown(e) && !e.isShiftDown() && av
                      .getSelectionGroup() != null))
      {
        av.setSelectionGroup(new SequenceGroup());
        av.getSelectionGroup().setEndRes(av.getAlignment().getWidth() - 1);
      }
  
+     int seq = alignPanel.getSeqPanel().findSeq(e);
      if (e.isShiftDown() && (lastid != -1))
      {
        selectSeqs(lastid, seq);
      {
        selectSeq(seq);
      }
-     // TODO is this addition ok here?
      av.isSelectionGroupChanged(true);
  
      alignPanel.paintAlignment(true);
    }
  
    /**
+    * Build and show the popup-menu at the right-click mouse position
+    * 
+    * @param e
+    */
+   void showPopupMenu(MouseEvent e)
+   {
+     int seq2 = alignPanel.getSeqPanel().findSeq(e);
+     Sequence sq = (Sequence) av.getAlignment().getSequenceAt(seq2);
+     // build a new links menu based on the current links + any non-positional
+     // features
+     Vector<String> nlinks = new Vector<String>(Preferences.sequenceURLLinks);
+     SequenceFeature sfs[] = sq == null ? null : sq.getSequenceFeatures();
+     if (sfs != null)
+     {
+       for (SequenceFeature sf : sfs)
+       {
+         if (sf.begin == sf.end && sf.begin == 0)
+         {
+           if (sf.links != null && sf.links.size() > 0)
+           {
+             for (int l = 0, lSize = sf.links.size(); l < lSize; l++)
+             {
+               nlinks.addElement(sf.links.elementAt(l));
+             }
+           }
+         }
+       }
+     }
+     PopupMenu pop = new PopupMenu(alignPanel, sq, nlinks,
+             Preferences.getGroupURLLinks());
+     pop.show(this, e.getX(), e.getY());
+   }
+   /**
     * Toggle whether the sequence is part of the current selection group.
     * 
     * @param seq
      PaintRefresher.Refresh(this, av.getSequenceSetId());
      // always send selection message when mouse is released
      av.sendSelection();
+     if (e.isPopupTrigger()) // Windows reports this in mouseReleased
+     {
+       showPopupMenu(e);
+     }
    }
  
    /**
@@@ -24,6 -24,7 +24,7 @@@ import jalview.analysis.AAFrequency
  import jalview.analysis.AlignmentAnnotationUtils;
  import jalview.analysis.AlignmentUtils;
  import jalview.analysis.Conservation;
+ import jalview.bin.Cache;
  import jalview.commands.ChangeCaseCommand;
  import jalview.commands.EditCommand;
  import jalview.commands.EditCommand.Action;
@@@ -48,12 -49,12 +49,12 @@@ import jalview.schemes.HydrophobicColou
  import jalview.schemes.NucleotideColourScheme;
  import jalview.schemes.PIDColourScheme;
  import jalview.schemes.PurinePyrimidineColourScheme;
- import jalview.schemes.ResidueProperties;
  import jalview.schemes.StrandColourScheme;
  import jalview.schemes.TaylorColourScheme;
  import jalview.schemes.TurnColourScheme;
  import jalview.schemes.UserColourScheme;
  import jalview.schemes.ZappoColourScheme;
+ import jalview.util.DBRefUtils;
  import jalview.util.GroupUrlLink;
  import jalview.util.GroupUrlLink.UrlStringTooLongException;
  import jalview.util.MessageManager;
@@@ -62,6 -63,7 +63,7 @@@ import jalview.util.UrlLink
  import java.awt.Color;
  import java.awt.event.ActionEvent;
  import java.awt.event.ActionListener;
+ import java.util.ArrayList;
  import java.util.Arrays;
  import java.util.Collections;
  import java.util.Hashtable;
@@@ -88,10 -90,6 +90,6 @@@ import javax.swing.JRadioButtonMenuItem
   */
  public class PopupMenu extends JPopupMenu
  {
-   private static final String ALL_ANNOTATIONS = "All";
-   private static final String COMMA = ",";
    JMenu groupMenu = new JMenu();
  
    JMenuItem groupName = new JMenuItem();
     * @param seq
     *          DOCUMENT ME!
     */
-   public PopupMenu(final AlignmentPanel ap, Sequence seq, Vector links)
+   public PopupMenu(final AlignmentPanel ap, Sequence seq, List<String> links)
    {
      this(ap, seq, links, null);
    }
     * @param groupLinks
     */
    public PopupMenu(final AlignmentPanel ap, final SequenceI seq,
-           Vector links, Vector groupLinks)
+           List<String> links, List<String> groupLinks)
    {
      // /////////////////////////////////////////////////////////
      // If this is activated from the sequence panel, the user may want to
  
      if (sg != null && sg.getSize() > 0)
      {
-       groupName.setText(MessageManager.formatMessage("label.name_param",
-               new Object[] { sg.getName() }));
        groupName.setText(MessageManager
                .getString("label.edit_name_and_description_current_group"));
  
  
      if (links != null && links.size() > 0)
      {
+       addFeatureLinks(seq, links);
+     }
+   }
  
-       JMenu linkMenu = new JMenu(MessageManager.getString("action.link"));
-       Vector linkset = new Vector();
-       for (int i = 0; i < links.size(); i++)
+   /**
+    * Adds a 'Link' menu item with a sub-menu item for each hyperlink provided.
+    * 
+    * @param seq
+    * @param links
+    */
+   void addFeatureLinks(final SequenceI seq, List<String> links)
+   {
+     JMenu linkMenu = new JMenu(MessageManager.getString("action.link"));
+     List<String> linkset = new ArrayList<String>();
+     for (String link : links)
+     {
+       UrlLink urlLink = null;
+       try
        {
-         String link = links.elementAt(i).toString();
-         UrlLink urlLink = null;
-         try
-         {
-           urlLink = new UrlLink(link);
-         } catch (Exception foo)
-         {
-           jalview.bin.Cache.log.error("Exception for URLLink '" + link
-                   + "'", foo);
-           continue;
-         }
-         ;
-         if (!urlLink.isValid())
-         {
-           jalview.bin.Cache.log.error(urlLink.getInvalidMessage());
-           continue;
-         }
-         final String label = urlLink.getLabel();
-         if (seq != null && urlLink.isDynamic())
-         {
+         urlLink = new UrlLink(link);
+       } catch (Exception foo)
+       {
+         Cache.log.error("Exception for URLLink '" + link + "'", foo);
+         continue;
+       }
+       ;
+       if (!urlLink.isValid())
+       {
+         Cache.log.error(urlLink.getInvalidMessage());
+         continue;
+       }
+       final String label = urlLink.getLabel();
  
-           // collect matching db-refs
-           DBRefEntry[] dbr = jalview.util.DBRefUtils.selectRefs(
-                   seq.getDBRefs(), new String[] { urlLink.getTarget() });
-           // collect id string too
-           String id = seq.getName();
-           String descr = seq.getDescription();
-           if (descr != null && descr.length() < 1)
-           {
-             descr = null;
-           }
+       // collect id string too
+       String id = seq.getName();
+       String descr = seq.getDescription();
+       if (descr != null && descr.length() < 1)
+       {
+         descr = null;
+       }
  
-           if (dbr != null)
-           {
-             for (int r = 0; r < dbr.length; r++)
-             {
-               if (id != null && dbr[r].getAccessionId().equals(id))
-               {
-                 // suppress duplicate link creation for the bare sequence ID
-                 // string with this link
-                 id = null;
-               }
-               // create Bare ID link for this RUL
-               String[] urls = urlLink.makeUrls(dbr[r].getAccessionId(),
-                       true);
-               if (urls != null)
-               {
-                 for (int u = 0; u < urls.length; u += 2)
-                 {
-                   if (!linkset.contains(urls[u] + "|" + urls[u + 1]))
-                   {
-                     linkset.addElement(urls[u] + "|" + urls[u + 1]);
-                     addshowLink(linkMenu, label + "|" + urls[u],
-                             urls[u + 1]);
-                   }
-                 }
-               }
-             }
-           }
-           if (id != null)
-           {
-             // create Bare ID link for this RUL
-             String[] urls = urlLink.makeUrls(id, true);
-             if (urls != null)
-             {
-               for (int u = 0; u < urls.length; u += 2)
-               {
-                 if (!linkset.contains(urls[u] + "|" + urls[u + 1]))
-                 {
-                   linkset.addElement(urls[u] + "|" + urls[u + 1]);
-                   addshowLink(linkMenu, label, urls[u + 1]);
-                 }
-               }
-             }
-           }
-           // Create urls from description but only for URL links which are regex
-           // links
-           if (descr != null && urlLink.getRegexReplace() != null)
+       if (seq != null && urlLink.usesSeqId()) // link is ID
+       {
+         // collect matching db-refs
+         DBRefEntry[] dbr = DBRefUtils.selectRefs(seq.getDBRefs(),
+                 new String[] { urlLink.getTarget() });
+         // if there are any dbrefs which match up with the link
+         if (dbr != null)
+         {
+           for (int r = 0; r < dbr.length; r++)
            {
-             // create link for this URL from description where regex matches
-             String[] urls = urlLink.makeUrls(descr, true);
-             if (urls != null)
+             if (id != null && dbr[r].getAccessionId().equals(id))
              {
-               for (int u = 0; u < urls.length; u += 2)
-               {
-                 if (!linkset.contains(urls[u] + "|" + urls[u + 1]))
-                 {
-                   linkset.addElement(urls[u] + "|" + urls[u + 1]);
-                   addshowLink(linkMenu, label, urls[u + 1]);
-                 }
-               }
+               // suppress duplicate link creation for the bare sequence ID
+               // string with this link
+               id = null;
              }
+             // create Bare ID link for this URL
+             createBareURLLink(urlLink, dbr[r].getAccessionId(), linkset,
+                     linkMenu, label, true);
            }
          }
-         else
+         // Create urls from description but only for URL links which are regex
+         // links
+         if (descr != null && urlLink.getRegexReplace() != null)
          {
-           if (!linkset.contains(label + "|" + urlLink.getUrl_prefix()))
-           {
-             linkset.addElement(label + "|" + urlLink.getUrl_prefix());
-             // Add a non-dynamic link
-             addshowLink(linkMenu, label, urlLink.getUrl_prefix());
-           }
+           // create link for this URL from description where regex matches
+           createBareURLLink(urlLink, descr, linkset, linkMenu, label, false);
          }
        }
-       if (sequence != null)
+       else if (seq != null && !urlLink.usesSeqId()) // link is name
        {
-         sequenceMenu.add(linkMenu);
+         if (id != null)
+         {
+           // create Bare ID link for this URL
+           createBareURLLink(urlLink, id, linkset, linkMenu, label, false);
+         }
+         // Create urls from description but only for URL links which are regex
+         // links
+         if (descr != null && urlLink.getRegexReplace() != null)
+         {
+           // create link for this URL from description where regex matches
+           createBareURLLink(urlLink, descr, linkset, linkMenu, label, false);
+         }
        }
        else
        {
-         add(linkMenu);
+         if (!linkset.contains(label + "|" + urlLink.getUrl_prefix()))
+         {
+           linkset.add(label + "|" + urlLink.getUrl_prefix());
+           // Add a non-dynamic link
+           addshowLink(linkMenu, label, urlLink.getUrl_prefix());
+         }
+       }
+     }
+     if (sequence != null)
+     {
+       sequenceMenu.add(linkMenu);
+     }
+     else
+     {
+       add(linkMenu);
+     }
+   }
+   /*
+    * Create a bare URL Link
+    */
+   private void createBareURLLink(UrlLink urlLink, String id,
+           List<String> linkset, JMenu linkMenu, String label,
+           Boolean addSepToLabel)
+   {
+     String[] urls = urlLink.makeUrls(id, true);
+     if (urls != null)
+     {
+       for (int u = 0; u < urls.length; u += 2)
+       {
+         if (!linkset.contains(urls[u] + "|" + urls[u + 1]))
+         {
+           linkset.add(urls[u] + "|" + urls[u + 1]);
+           if (addSepToLabel)
+           {
+             addshowLink(linkMenu, label + "|" + urls[u], urls[u + 1]);
+           }
+           else
+           {
+             addshowLink(linkMenu, label, urls[u + 1]);
+           }
+         }
        }
      }
    }
      showMenu.removeAll();
      hideMenu.removeAll();
  
-     final List<String> all = Arrays.asList(ALL_ANNOTATIONS);
+     final List<String> all = Arrays.asList(new String[] { MessageManager
+             .getString("label.all") });
      addAnnotationTypeToShowHide(showMenu, forSequences, "", all, true, true);
      addAnnotationTypeToShowHide(hideMenu, forSequences, "", all, true,
              false);
      showOrHideMenu.add(item);
    }
  
-   private void buildGroupURLMenu(SequenceGroup sg, Vector groupLinks)
+   private void buildGroupURLMenu(SequenceGroup sg, List<String> groupLinks)
    {
  
      // TODO: usability: thread off the generation of group url content so root
      // ID/regex match URLs
      groupLinksMenu = new JMenu(
              MessageManager.getString("action.group_link"));
+     // three types of url that might be created.
      JMenu[] linkMenus = new JMenu[] { null,
          new JMenu(MessageManager.getString("action.ids")),
          new JMenu(MessageManager.getString("action.sequences")),
-         new JMenu(MessageManager.getString("action.ids_sequences")) }; // three
-                                                                        // types
-                                                                        // of url
-                                                                        // that
-                                                                        // might
-                                                                        // be
-     // created.
+         new JMenu(MessageManager.getString("action.ids_sequences")) };
      SequenceI[] seqs = ap.av.getSelectionAsNewSequence();
      String[][] idandseqs = GroupUrlLink.formStrings(seqs);
-     Hashtable commonDbrefs = new Hashtable();
+     Hashtable<String, Object[]> commonDbrefs = new Hashtable<String, Object[]>();
      for (int sq = 0; sq < seqs.length; sq++)
      {
  
          for (int d = 0; d < dbr.length; d++)
          {
            String src = dbr[d].getSource(); // jalview.util.DBRefUtils.getCanonicalName(dbr[d].getSource()).toUpperCase();
-           Object[] sarray = (Object[]) commonDbrefs.get(src);
+           Object[] sarray = commonDbrefs.get(src);
            if (sarray == null)
            {
              sarray = new Object[2];
      // now create group links for all distinct ID/sequence sets.
      boolean addMenu = false; // indicates if there are any group links to give
                               // to user
-     for (int i = 0; i < groupLinks.size(); i++)
+     for (String link : groupLinks)
      {
-       String link = groupLinks.elementAt(i).toString();
        GroupUrlLink urlLink = null;
        try
        {
          urlLink = new GroupUrlLink(link);
        } catch (Exception foo)
        {
-         jalview.bin.Cache.log.error("Exception for GroupURLLink '" + link
-                 + "'", foo);
+         Cache.log.error("Exception for GroupURLLink '" + link + "'", foo);
          continue;
        }
        ;
        if (!urlLink.isValid())
        {
-         jalview.bin.Cache.log.error(urlLink.getInvalidMessage());
+         Cache.log.error(urlLink.getInvalidMessage());
          continue;
        }
        final String label = urlLink.getLabel();
        boolean usingNames = false;
        // Now see which parts of the group apply for this URL
        String ltarget = urlLink.getTarget(); // jalview.util.DBRefUtils.getCanonicalName(urlLink.getTarget());
-       Object[] idset = (Object[]) commonDbrefs.get(ltarget.toUpperCase());
+       Object[] idset = commonDbrefs.get(ltarget.toUpperCase());
        String[] seqstr, ids; // input to makeUrl
        if (idset != null)
        {
              new Object[] { urlgenerator.getUrl_prefix(),
                  urlgenerator.getNumberInvolved(urlstub) }));
      // TODO: put in info about what is being sent.
-     item.addActionListener(new java.awt.event.ActionListener()
+     item.addActionListener(new ActionListener()
      {
        @Override
        public void actionPerformed(ActionEvent e)
              try
              {
                showLink(urlgenerator.constructFrom(urlstub));
-             } catch (UrlStringTooLongException e)
+             } catch (UrlStringTooLongException e2)
              {
              }
            }
     */
    private void jbInit() throws Exception
    {
-     groupMenu.setText(MessageManager.getString("label.group"));
      groupMenu.setText(MessageManager.getString("label.selection"));
      groupName.setText(MessageManager.getString("label.name"));
      groupName.addActionListener(new java.awt.event.ActionListener()
    public void createSequenceDetailsReport(SequenceI[] sequences)
    {
      CutAndPasteHtmlTransfer cap = new CutAndPasteHtmlTransfer();
 -    StringBuffer contents = new StringBuffer();
 +    StringBuilder contents = new StringBuilder(128);
      for (SequenceI seq : sequences)
      {
        contents.append("<p><h2>"
                        seq,
                        true,
                        true,
 -                      false,
                        (ap.getSeqPanel().seqCanvas.fr != null) ? ap
                                .getSeqPanel().seqCanvas.fr.getMinMax()
                                : null);
      if (conservationMenuItem.isSelected())
      {
        // JBPNote: Conservation name shouldn't be i18n translated
-       Conservation c = new Conservation("Group",
-               ResidueProperties.propHash, 3, sg.getSequences(ap.av
-                       .getHiddenRepSequences()), sg.getStartRes(),
+       Conservation c = new Conservation("Group", 3, sg.getSequences(ap.av
+               .getHiddenRepSequences()), sg.getStartRes(),
                sg.getEndRes() + 1);
  
        c.calculate();
  
    void hideSequences(boolean representGroup)
    {
-     SequenceGroup sg = ap.av.getSelectionGroup();
-     if (sg == null || sg.getSize() < 1)
-     {
-       ap.av.hideSequence(new SequenceI[] { sequence });
-       return;
-     }
-     ap.av.setSelectionGroup(null);
-     if (representGroup)
-     {
-       ap.av.hideRepSequences(sequence, sg);
-       return;
-     }
-     int gsize = sg.getSize();
-     SequenceI[] hseqs = sg.getSequences().toArray(new SequenceI[gsize]);
-     ap.av.hideSequence(hseqs);
-     // refresh(); TODO: ? needed ?
-     ap.av.sendSelection();
+     ap.av.hideSequences(sequence, representGroup);
    }
  
    public void copy_actionPerformed()
              ap, true));
    }
  
-   public void enterPDB_actionPerformed()
-   {
-     String id = JOptionPane.showInternalInputDialog(Desktop.desktop,
-             MessageManager.getString("label.enter_pdb_id"),
-             MessageManager.getString("label.enter_pdb_id"),
-             JOptionPane.QUESTION_MESSAGE);
-     if (id != null && id.length() > 0)
-     {
-       PDBEntry entry = new PDBEntry();
-       entry.setId(id.toUpperCase());
-       sequence.getDatasetSequence().addPDBId(entry);
-     }
-   }
-   public void discoverPDB_actionPerformed()
-   {
-     final SequenceI[] sequences = ((ap.av.getSelectionGroup() == null) ? new SequenceI[]
-     { sequence }
-             : ap.av.getSequenceSelection());
-     Thread discpdb = new Thread(new Runnable()
-     {
-       @Override
-       public void run()
-       {
-         boolean isNuclueotide = ap.alignFrame.getViewport().getAlignment()
-                 .isNucleotide();
-         new jalview.ws.DBRefFetcher(sequences, ap.alignFrame, null,
-                 ap.alignFrame.featureSettings, isNuclueotide)
-                 .fetchDBRefs(false);
-       }
-     });
-     discpdb.start();
-   }
    public void sequenceFeature_actionPerformed()
    {
      SequenceGroup sg = ap.av.getSelectionGroup();
@@@ -21,6 -21,7 +21,7 @@@
  package jalview.gui;
  
  import jalview.api.AlignViewportI;
+ import jalview.bin.Cache;
  import jalview.commands.EditCommand;
  import jalview.commands.EditCommand.Action;
  import jalview.commands.EditCommand.Edit;
@@@ -42,6 -43,7 +43,7 @@@ import jalview.structure.VamsasSource
  import jalview.util.Comparison;
  import jalview.util.MappingUtils;
  import jalview.util.MessageManager;
+ import jalview.util.Platform;
  import jalview.viewmodel.AlignmentViewport;
  
  import java.awt.BorderLayout;
@@@ -54,11 -56,12 +56,12 @@@ import java.awt.event.MouseListener
  import java.awt.event.MouseMotionListener;
  import java.awt.event.MouseWheelEvent;
  import java.awt.event.MouseWheelListener;
+ import java.util.ArrayList;
  import java.util.List;
  
  import javax.swing.JOptionPane;
  import javax.swing.JPanel;
+ import javax.swing.SwingUtilities;
  import javax.swing.ToolTipManager;
  
  /**
@@@ -121,7 -124,7 +124,7 @@@ public class SeqPanel extends JPanel im
  
    private final SequenceAnnotationReport seqARep;
  
 -  StringBuffer tooltipText = new StringBuffer();
 +  StringBuilder tooltipText = new StringBuilder();
  
    String tmpString;
  
  
    void insertNucAtCursor(boolean group, String nuc)
    {
+     // TODO not called - delete?
      groupEditing = group;
      startseq = seqCanvas.cursorY;
      lastres = seqCanvas.cursorX;
      mouseDragging = false;
      mouseWheelPressed = false;
  
+     if (evt.isPopupTrigger()) // Windows: mouseReleased
+     {
+       showPopupMenu(evt);
+       evt.consume();
+       return;
+     }
      if (!editingSeqs)
      {
        doMouseReleasedDefineMode(evt);
    {
      lastMousePress = evt.getPoint();
  
-     if (javax.swing.SwingUtilities.isMiddleMouseButton(evt))
+     if (SwingUtilities.isMiddleMouseButton(evt))
      {
        mouseWheelPressed = true;
        return;
      }
  
-     if (evt.isShiftDown() || evt.isAltDown() || evt.isControlDown())
+     boolean isControlDown = Platform.isControlDown(evt);
+     if (evt.isShiftDown() || isControlDown)
      {
-       if (evt.isAltDown() || evt.isControlDown())
+       editingSeqs = true;
+       if (isControlDown)
        {
          groupEditing = true;
        }
-       editingSeqs = true;
      }
      else
      {
    String lastTooltip;
  
    /**
+    * set when the current UI interaction has resulted in a change that requires
+    * overview shading to be recalculated. this could be changed to something
+    * more expressive that indicates what actually has changed, so selective
+    * redraws can be applied
+    */
+   private boolean needOverviewUpdate = false; // TODO: refactor to avcontroller
+   /**
+    * set if av.getSelectionGroup() refers to a group that is defined on the
+    * alignment view, rather than a transient selection
+    */
+   // private boolean editingDefinedGroup = false; // TODO: refactor to
+   // avcontroller or viewModel
+   /**
     * Set status message in alignment panel
     * 
     * @param sequence
       * Sequence number (if known), and sequence name.
       */
      String seqno = seq == -1 ? "" : " " + (seq + 1);
-     text.append("Sequence" + seqno + " ID: " + sequence.getName());
+     text.append("Sequence").append(seqno).append(" ID: ")
+             .append(sequence.getName());
  
      String residue = null;
      /*
            {
              for (int j = 0; j < startres - lastres; j++)
              {
-               if (!jalview.util.Comparison.isGap(groupSeqs[g]
-                       .getCharAt(fixedRight - j)))
+               if (!Comparison.isGap(groupSeqs[g].getCharAt(fixedRight - j)))
                {
                  blank = false;
                  break;
                continue;
              }
  
-             if (!jalview.util.Comparison.isGap(groupSeqs[g].getCharAt(j)))
+             if (!Comparison.isGap(groupSeqs[g].getCharAt(j)))
              {
                // Not a gap, block edit not valid
                endEditing();
  
        for (blankColumn = fixedColumn; blankColumn > j; blankColumn--)
        {
-         if (jalview.util.Comparison.isGap(seq[s].getCharAt(blankColumn)))
+         if (Comparison.isGap(seq[s].getCharAt(blankColumn)))
          {
            // Theres a space, so break and insert the gap
            break;
     */
    public void doMousePressedDefineMode(MouseEvent evt)
    {
-     int res = findRes(evt);
-     int seq = findSeq(evt);
+     final int res = findRes(evt);
+     final int seq = findSeq(evt);
      oldSeq = seq;
+     needOverviewUpdate = false;
  
      startWrapBlock = wrappedBlock;
  
        }
  
        av.setSelectionGroup(stretchGroup);
      }
  
-     if (javax.swing.SwingUtilities.isRightMouseButton(evt))
+     if (evt.isPopupTrigger()) // Mac: mousePressed
      {
-       List<SequenceFeature> allFeatures = ap.getFeatureRenderer()
-               .findFeaturesAtRes(sequence.getDatasetSequence(),
-                       sequence.findPosition(res));
-       Vector links = new Vector();
-       for (SequenceFeature sf : allFeatures)
-       {
-         if (sf.links != null)
-         {
-           for (int j = 0; j < sf.links.size(); j++)
-           {
-             links.addElement(sf.links.elementAt(j));
-           }
-         }
-       }
+       showPopupMenu(evt);
+       return;
+     }
  
-       jalview.gui.PopupMenu pop = new jalview.gui.PopupMenu(ap, null, links);
-       pop.show(this, evt.getX(), evt.getY());
+     /*
+      * defer right-mouse click handling to mouseReleased on Windows
+      * (where isPopupTrigger() will answer true)
+      * NB isRightMouseButton is also true for Cmd-click on Mac
+      */
+     if (SwingUtilities.isRightMouseButton(evt) && !Platform.isAMac())
+     {
        return;
      }
  
        sg.setEndRes(res);
        sg.addSequence(sequence, false);
        av.setSelectionGroup(sg);
        stretchGroup = sg;
  
        if (av.getConservationSelected())
    }
  
    /**
+    * Build and show a pop-up menu at the right-click mouse position
+    * 
+    * @param evt
+    * @param res
+    * @param sequence
+    */
+   void showPopupMenu(MouseEvent evt)
+   {
+     final int res = findRes(evt);
+     final int seq = findSeq(evt);
+     SequenceI sequence = av.getAlignment().getSequenceAt(seq);
+     List<SequenceFeature> allFeatures = ap.getFeatureRenderer()
+             .findFeaturesAtRes(sequence.getDatasetSequence(),
+                     sequence.findPosition(res));
+     List<String> links = new ArrayList<String>();
+     for (SequenceFeature sf : allFeatures)
+     {
+       if (sf.links != null)
+       {
+         for (String link : sf.links)
+         {
+           links.add(link);
+         }
+       }
+     }
+     PopupMenu pop = new PopupMenu(ap, null, links);
+     pop.show(this, evt.getX(), evt.getY());
+   }
+   /**
     * DOCUMENT ME!
     * 
     * @param evt
      {
        return;
      }
-     stretchGroup.recalcConservation(); // always do this - annotation has own
-                                        // state
+     // always do this - annotation has own state
+     // but defer colourscheme update until hidden sequences are passed in
+     boolean vischange = stretchGroup.recalcConservation(true);
+     needOverviewUpdate |= vischange && av.isSelectionDefinedGroup();
      if (stretchGroup.cs != null)
      {
        stretchGroup.cs.alignmentChanged(stretchGroup,
        }
      }
      PaintRefresher.Refresh(this, av.getSequenceSetId());
-     ap.paintAlignment(true);
+     ap.paintAlignment(needOverviewUpdate);
+     needOverviewUpdate = false;
      changeEndRes = false;
      changeStartRes = false;
      stretchGroup = null;
        if (res > (stretchGroup.getStartRes() - 1))
        {
          stretchGroup.setEndRes(res);
+         needOverviewUpdate |= av.isSelectionDefinedGroup();
        }
      }
      else if (changeStartRes)
        if (res < (stretchGroup.getEndRes() + 1))
        {
          stretchGroup.setStartRes(res);
+         needOverviewUpdate |= av.isSelectionDefinedGroup();
        }
      }
  
        if (stretchGroup.getSequences(null).contains(nextSeq))
        {
          stretchGroup.deleteSequence(seq, false);
+         needOverviewUpdate |= av.isSelectionDefinedGroup();
        }
        else
        {
          }
  
          stretchGroup.addSequence(nextSeq, false);
+         needOverviewUpdate |= av.isSelectionDefinedGroup();
        }
      }
  
  
      // do we want to thread this ? (contention with seqsel and colsel locks, I
      // suspect)
-     // rules are: colsel is copied if there is a real intersection between
-     // sequence selection
+     /*
+      * only copy colsel if there is a real intersection between
+      * sequence selection and this panel's alignment
+      */
      boolean repaint = false;
-     boolean copycolsel = true;
+     boolean copycolsel = false;
  
      SequenceGroup sgroup = null;
      if (seqsel != null && seqsel.getSize() > 0)
      {
        if (av.getAlignment() == null)
        {
-         jalview.bin.Cache.log.warn("alignviewport av SeqSetId="
-                 + av.getSequenceSetId() + " ViewId=" + av.getViewId()
+         Cache.log.warn("alignviewport av SeqSetId=" + av.getSequenceSetId()
+                 + " ViewId=" + av.getViewId()
                  + " 's alignment is NULL! returning immediately.");
          return;
        }
        sgroup = seqsel.intersect(av.getAlignment(),
                (av.hasHiddenRows()) ? av.getHiddenRepSequences() : null);
-       if ((sgroup == null || sgroup.getSize() == 0)
-               || (colsel == null || colsel.isEmpty()))
+       if ((sgroup != null && sgroup.getSize() > 0))
        {
-         // don't copy columns if the region didn't intersect.
-         copycolsel = false;
+         copycolsel = true;
        }
      }
      if (sgroup != null && sgroup.getSize() > 0)
      ColumnSelection cs = MappingUtils.mapColumnSelection(colsel, sourceAv,
              av);
      av.setColumnSelection(cs);
-     av.isColSelChanged(true);
  
      PaintRefresher.Refresh(this, av.getSequenceSetId());
  
  package jalview.io;
  
  import jalview.datamodel.DBRefEntry;
++import jalview.datamodel.DBRefSource;
  import jalview.datamodel.SequenceFeature;
  import jalview.datamodel.SequenceI;
+ import jalview.io.gff.GffConstants;
+ import jalview.util.DBRefUtils;
++import jalview.util.MessageManager;
  import jalview.util.UrlLink;
  
  import java.util.ArrayList;
 +import java.util.Arrays;
- import java.util.Collections;
 +import java.util.Comparator;
- import java.util.Hashtable;
  import java.util.List;
+ import java.util.Map;
  
  /**
   * generate HTML reports for a sequence
   */
  public class SequenceAnnotationReport
  {
++  private static final String COMMA = ",";
++
++  private static final String ELLIPSIS = "...";
++
++  private static final int MAX_REFS_PER_SOURCE = 4;
++
++  private static final int MAX_SOURCES = 40;
++
++  private static final String[][] PRIMARY_SOURCES = new String[][] {
++      DBRefSource.CODINGDBS, DBRefSource.DNACODINGDBS,
++      DBRefSource.PROTEINDBS };
++
    final String linkImageURL;
  
 +  /*
 +   * Comparator to order DBRefEntry by Source + accession id (case-insensitive)
 +   */
 +  private static Comparator<DBRefEntry> comparator = new Comparator<DBRefEntry>()
 +  {
++
 +    @Override
 +    public int compare(DBRefEntry ref1, DBRefEntry ref2)
 +    {
 +      String s1 = ref1.getSource();
 +      String s2 = ref2.getSource();
++      boolean s1Primary = isPrimarySource(s1);
++      boolean s2Primary = isPrimarySource(s2);
++      if (s1Primary && !s2Primary)
++      {
++        return -1;
++      }
++      if (!s1Primary && s2Primary)
++      {
++        return 1;
++      }
 +      int comp = s1 == null ? -1 : (s2 == null ? 1 : s1
 +              .compareToIgnoreCase(s2));
 +      if (comp == 0)
 +      {
 +        String a1 = ref1.getAccessionId();
 +        String a2 = ref2.getAccessionId();
 +        comp = a1 == null ? -1 : (a2 == null ? 1 : a1
 +                .compareToIgnoreCase(a2));
 +      }
 +      return comp;
 +    }
++
++    private boolean isPrimarySource(String source)
++    {
++      for (String[] primary : PRIMARY_SOURCES)
++      {
++        for (String s : primary)
++        {
++          if (source.equals(s))
++          {
++            return true;
++          }
++        }
++      }
++      return false;
++    }
 +  };
 +
    public SequenceAnnotationReport(String linkImageURL)
    {
      this.linkImageURL = linkImageURL;
    }
  
    /**
-    * appends the features at rpos to the given stringbuffer ready for display in
-    * a tooltip
+    * Append text for the list of features to the tooltip
     * 
-    * @param tooltipText
-    * @param linkImageURL
 -   * @param tooltipText2
++   * @param sb
     * @param rpos
     * @param features
     * @param minmax
-    *          TODO refactor to Jalview 'utilities' somehow.
     */
-   public void appendFeatures(final StringBuilder tooltipText, int rpos,
-           List<SequenceFeature> features, Hashtable minmax)
 -  public void appendFeatures(final StringBuffer tooltipText2, int rpos,
++  public void appendFeatures(final StringBuilder sb, int rpos,
+           List<SequenceFeature> features, Map<String, float[][]> minmax)
    {
-     String tmpString;
      if (features != null)
      {
        for (SequenceFeature feature : features)
        {
-         if (feature.getType().equals("disulfide bond"))
 -        appendFeature(tooltipText2, rpos, minmax, feature);
++        appendFeature(sb, rpos, minmax, feature);
+       }
+     }
+   }
+   /**
 -   * Appends text for one sequence feature to the string buffer
++   * Appends the feature at rpos to the given buffer
+    * 
+    * @param sb
+    * @param rpos
+    * @param minmax
 -   *          {{min, max}, {min, max}} positional and non-positional feature
 -   *          scores for this type
+    * @param feature
+    */
 -  void appendFeature(final StringBuffer sb, int rpos,
++  void appendFeature(final StringBuilder sb, int rpos,
+           Map<String, float[][]> minmax, SequenceFeature feature)
+   {
 -    if ("disulfide bond".equals(feature.getType()))
++    String tmpString;
++    if (feature.getType().equals("disulfide bond"))
+     {
+       if (feature.getBegin() == rpos || feature.getEnd() == rpos)
+       {
+         if (sb.length() > 6)
          {
-           if (feature.getBegin() == rpos || feature.getEnd() == rpos)
-           {
-             if (tooltipText.length() > 6)
-             {
-               tooltipText.append("<br>");
-             }
-             tooltipText.append("disulfide bond " + feature.getBegin()
-                     + ":" + feature.getEnd());
-           }
+           sb.append("<br>");
          }
-         else
+         sb.append("disulfide bond ").append(feature.getBegin()).append(":")
+                 .append(feature.getEnd());
+       }
+     }
+     else
+     {
+       if (sb.length() > 6)
+       {
+         sb.append("<br>");
+       }
+       // TODO: remove this hack to display link only features
+       boolean linkOnly = feature.getValue("linkonly") != null;
+       if (!linkOnly)
+       {
+         sb.append(feature.getType()).append(" ");
+         if (rpos != 0)
          {
-           if (tooltipText.length() > 6)
+           // we are marking a positional feature
+           sb.append(feature.begin);
+         }
+         if (feature.begin != feature.end)
+         {
 -          sb.append(" " + feature.end);
++          sb.append(" ").append(feature.end);
+         }
+         if (feature.getDescription() != null
+                 && !feature.description.equals(feature.getType()))
+         {
 -          String tmpString = feature.getDescription();
++          tmpString = feature.getDescription();
+           String tmp2up = tmpString.toUpperCase();
 -          final int startTag = tmp2up.indexOf("<HTML>");
++          int startTag = tmp2up.indexOf("<HTML>");
+           if (startTag > -1)
            {
-             tooltipText.append("<br>");
+             tmpString = tmpString.substring(startTag + 6);
+             tmp2up = tmp2up.substring(startTag + 6);
            }
-           // TODO: remove this hack to display link only features
-           boolean linkOnly = feature.getValue("linkonly") != null;
-           if (!linkOnly)
 -          // TODO strips off </body> but not <body> - is that intended?
+           int endTag = tmp2up.indexOf("</BODY>");
+           if (endTag > -1)
            {
-             tooltipText.append(feature.getType() + " ");
-             if (rpos != 0)
-             {
-               // we are marking a positional feature
-               tooltipText.append(feature.begin);
-             }
-             if (feature.begin != feature.end)
-             {
-               tooltipText.append(" " + feature.end);
-             }
+             tmpString = tmpString.substring(0, endTag);
+             tmp2up = tmp2up.substring(0, endTag);
+           }
+           endTag = tmp2up.indexOf("</HTML>");
+           if (endTag > -1)
+           {
+             tmpString = tmpString.substring(0, endTag);
+           }
  
-             if (feature.getDescription() != null
-                     && !feature.description.equals(feature.getType()))
+           if (startTag > -1)
+           {
+             sb.append("; ").append(tmpString);
+           }
+           else
+           {
+             if (tmpString.indexOf("<") > -1 || tmpString.indexOf(">") > -1)
              {
-               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)
-               {
-                 tooltipText.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;");
+               // 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("; ").append(tmpString);
 +
-                   tooltipText.append("; ");
-                   tooltipText.append(tmpString);
-                 }
-                 else
-                 {
-                   tooltipText.append("; " + tmpString);
-                 }
-               }
++              sb.append("; ");
++              sb.append(tmpString);
              }
-             // check score should be shown
-             if (!Float.isNaN(feature.getScore()))
+             else
              {
-               float[][] rng = (minmax == null) ? null : ((float[][]) minmax
-                       .get(feature.getType()));
-               if (rng != null && rng[0] != null && rng[0][0] != rng[0][1])
-               {
-                 tooltipText.append(" Score=" + feature.getScore());
-               }
-             }
-             if (feature.getValue("status") != null)
-             {
-               String status = feature.getValue("status").toString();
-               if (status.length() > 0)
-               {
-                 tooltipText.append("; (" + feature.getValue("status")
-                         + ")");
-               }
+               sb.append("; ").append(tmpString);
              }
            }
          }
-         if (feature.links != null)
 -
 -        /*
 -         * score should be shown if there is one, and min != max
 -         * for this feature type (e.g. not all 0)
 -         */
++        // check score should be shown
+         if (!Float.isNaN(feature.getScore()))
          {
-           if (linkImageURL != null)
 -          float[][] rng = (minmax == null) ? null : minmax.get(feature
 -                  .getType());
++          float[][] rng = (minmax == null) ? null : ((float[][]) minmax
++                  .get(feature.getType()));
+           if (rng != null && rng[0] != null && rng[0][0] != rng[0][1])
            {
-             tooltipText.append(" <img src=\"" + linkImageURL + "\">");
 -            sb.append(" Score=").append(String.valueOf(feature.getScore()));
++            sb.append(" Score=" + feature.getScore());
            }
-           else
+         }
+         String status = (String) feature.getValue("status");
+         if (status != null && status.length() > 0)
+         {
+           sb.append("; (").append(status).append(")");
+         }
+         String clinSig = (String) feature
+                 .getValue(GffConstants.CLINICAL_SIGNIFICANCE);
+         if (clinSig != null)
+         {
+           sb.append("; ").append(clinSig);
+         }
+       }
+     }
 -    appendLinks(sb, feature);
+   }
+   /**
+    * Format and appends any hyperlinks for the sequence feature to the string
+    * buffer
+    * 
+    * @param sb
+    * @param feature
+    */
+   void appendLinks(final StringBuffer sb, SequenceFeature feature)
+   {
+     if (feature.links != null)
+     {
+       if (linkImageURL != null)
+       {
+         sb.append(" <img src=\"" + linkImageURL + "\">");
+       }
+       else
+       {
+         for (String urlstring : feature.links)
+         {
+           try
            {
-             for (String urlstring : feature.links)
+             for (String[] urllink : createLinksFrom(null, urlstring))
              {
-               try
-               {
-                 for (String[] urllink : createLinksFrom(null, urlstring))
-                 {
-                   tooltipText.append("<br/> <a href=\""
-                           + urllink[3]
-                           + "\" target=\""
-                           + urllink[0]
-                           + "\">"
-                           + (urllink[0].toLowerCase().equals(
-                                   urllink[1].toLowerCase()) ? urllink[0]
-                                   : (urllink[0] + ":" + urllink[1]))
-                           + "</a></br>");
-                 }
-               } catch (Exception x)
-               {
-                 System.err.println("problem when creating links from "
-                         + urlstring);
-                 x.printStackTrace();
-               }
+               sb.append("<br/> <a href=\""
+                       + urllink[3]
+                       + "\" target=\""
+                       + urllink[0]
+                       + "\">"
+                       + (urllink[0].toLowerCase().equals(
+                               urllink[1].toLowerCase()) ? urllink[0]
+                               : (urllink[0] + ":" + urllink[1]))
+                       + "</a></br>");
              }
+           } catch (Exception x)
+           {
+             System.err.println("problem when creating links from "
+                     + urlstring);
+             x.printStackTrace();
            }
          }
        }
      }
    }
  
     * @return String[][] { String[] { link target, link label, dynamic component
     *         inserted (if any), url }}
     */
-   public String[][] createLinksFrom(SequenceI seq, String link)
+   String[][] createLinksFrom(SequenceI seq, String link)
    {
-     ArrayList<String[]> urlSets = new ArrayList<String[]>();
-     ArrayList<String> uniques = new ArrayList<String>();
+     List<String[]> urlSets = new ArrayList<String[]>();
+     List<String> uniques = new ArrayList<String>();
      UrlLink urlLink = new UrlLink(link);
      if (!urlLink.isValid())
      {
        System.err.println(urlLink.getInvalidMessage());
        return null;
      }
 +    final String target = urlLink.getTarget(); // link.substring(0,
 +    // link.indexOf("|"));
 +    final String label = urlLink.getLabel();
      if (seq != null && urlLink.isDynamic())
      {
-       // collect matching db-refs
-       DBRefEntry[] dbr = jalview.util.DBRefUtils.selectRefs(seq.getDBRefs(),
-               new String[] { target });
-       // collect id string too
-       String id = seq.getName();
-       String descr = seq.getDescription();
-       if (descr != null && descr.length() < 1)
+       urlSets.addAll(createDynamicLinks(seq, urlLink, uniques));
+     }
+     else
+     {
 -      String target = urlLink.getTarget();
 -      String label = urlLink.getLabel();
+       String unq = label + "|" + urlLink.getUrl_prefix();
+       if (!uniques.contains(unq))
        {
-         descr = null;
+         uniques.add(unq);
+         urlSets.add(new String[] { target, label, null,
+             urlLink.getUrl_prefix() });
        }
-       if (dbr != null)
+     }
+     return urlSets.toArray(new String[][] {});
+   }
+   /**
+    * Formats and returns a list of dynamic href links
+    * 
+    * @param seq
+    * @param urlLink
+    * @param uniques
+    */
+   List<String[]> createDynamicLinks(SequenceI seq, UrlLink urlLink,
+           List<String> uniques)
+   {
+     List<String[]> result = new ArrayList<String[]>();
+     final String target = urlLink.getTarget();
+     final String label = urlLink.getLabel();
+     // collect matching db-refs
+     DBRefEntry[] dbr = DBRefUtils.selectRefs(seq.getDBRefs(),
+             new String[] { target });
+     // collect id string too
+     String id = seq.getName();
+     String descr = seq.getDescription();
+     if (descr != null && descr.length() < 1)
+     {
+       descr = null;
+     }
+     if (dbr != null)
+     {
+       for (int r = 0; r < dbr.length; r++)
        {
-         for (int r = 0; r < dbr.length; r++)
+         if (id != null && dbr[r].getAccessionId().equals(id))
          {
-           if (id != null && dbr[r].getAccessionId().equals(id))
-           {
-             // suppress duplicate link creation for the bare sequence ID
-             // string with this link
-             id = null;
-           }
-           // create Bare ID link for this RUL
-           String[] urls = urlLink.makeUrls(dbr[r].getAccessionId(), true);
-           if (urls != null)
-           {
-             for (int u = 0; u < urls.length; u += 2)
-             {
-               String unq = urls[u] + "|" + urls[u + 1];
-               if (!uniques.contains(unq))
-               {
-                 urlSets.add(new String[] { target, label, urls[u],
-                     urls[u + 1] });
-                 uniques.add(unq);
-               }
-             }
-           }
+           // suppress duplicate link creation for the bare sequence ID
+           // string with this link
+           id = null;
          }
-       }
-       if (id != null)
-       {
-         // create Bare ID link for this RUL
-         String[] urls = urlLink.makeUrls(id, true);
+         // create Bare ID link for this URL
+         String[] urls = urlLink.makeUrls(dbr[r].getAccessionId(), true);
          if (urls != null)
          {
            for (int u = 0; u < urls.length; u += 2)
              String unq = urls[u] + "|" + urls[u + 1];
              if (!uniques.contains(unq))
              {
-               urlSets.add(new String[] { target, label, urls[u],
-                   urls[u + 1] });
+               result.add(new String[] { target, label, urls[u], urls[u + 1] });
                uniques.add(unq);
              }
            }
          }
        }
-       if (descr != null && urlLink.getRegexReplace() != null)
+     }
+     if (id != null)
+     {
+       // create Bare ID link for this URL
+       String[] urls = urlLink.makeUrls(id, true);
+       if (urls != null)
        {
-         // create link for this URL from description only if regex matches
-         String[] urls = urlLink.makeUrls(descr, true);
-         if (urls != null)
+         for (int u = 0; u < urls.length; u += 2)
          {
-           for (int u = 0; u < urls.length; u += 2)
+           String unq = urls[u] + "|" + urls[u + 1];
+           if (!uniques.contains(unq))
            {
-             String unq = urls[u] + "|" + urls[u + 1];
-             if (!uniques.contains(unq))
-             {
-               urlSets.add(new String[] { target, label, urls[u],
-                   urls[u + 1] });
-               uniques.add(unq);
-             }
+             result.add(new String[] { target, label, urls[u], urls[u + 1] });
+             uniques.add(unq);
            }
          }
        }
      }
-     else
+     if (descr != null && urlLink.getRegexReplace() != null)
      {
-       String unq = label + "|" + urlLink.getUrl_prefix();
-       if (!uniques.contains(unq))
+       // create link for this URL from description only if regex matches
+       String[] urls = urlLink.makeUrls(descr, true);
+       if (urls != null)
        {
-         uniques.add(unq);
-         // Add a non-dynamic link
-         urlSets.add(new String[] { target, label, null,
-             urlLink.getUrl_prefix() });
+         for (int u = 0; u < urls.length; u += 2)
+         {
+           String unq = urls[u] + "|" + urls[u + 1];
+           if (!uniques.contains(unq))
+           {
+             result.add(new String[] { target, label, urls[u], urls[u + 1] });
+             uniques.add(unq);
+           }
+         }
        }
      }
-     return urlSets.toArray(new String[][] {});
+     return result;
    }
  
-   public void createTooltipAnnotationReport(final StringBuilder tip,
 -  public void createSequenceAnnotationReport(final StringBuffer tip,
++  public void createSequenceAnnotationReport(final StringBuilder tip,
            SequenceI sequence, boolean showDbRefs, boolean showNpFeats,
-           Hashtable minmax)
+           Map<String, float[][]> minmax)
    {
-     int maxWidth = createSequenceAnnotationReport(tip, sequence,
-             showDbRefs, showNpFeats, minmax, true);
-     if (maxWidth > 60)
-     {
-       tip.insert(0, "<table width=350 border=0><tr><td><i>");
-       tip.append("</i></td></tr></table>");
-     }
-   }
-   public int createSequenceAnnotationReport(final StringBuilder tip,
-           SequenceI sequence, boolean showDbRefs, boolean showNpFeats,
-           Hashtable minmax)
-   {
-     return createSequenceAnnotationReport(tip, sequence, showDbRefs,
-             showNpFeats, minmax, false);
+     createSequenceAnnotationReport(tip, sequence, showDbRefs, showNpFeats,
 -            true, minmax);
++            minmax, false);
    }
  
 -  public void createSequenceAnnotationReport(final StringBuffer tip,
 +  /**
-    * Adds an html-formatted sequence annotation report to the provided string
-    * buffer, and returns the longest line length added
++   * Builds an html formatted report of sequence details and appends it to the
++   * provided buffer.
 +   * 
 +   * @param sb
++   *          buffer to append report to
 +   * @param sequence
++   *          the sequence the report is for
 +   * @param showDbRefs
-    *          if true, include database references
++   *          whether to include database references for the sequence
 +   * @param showNpFeats
-    *          if true, include non-positional sequence features
++   *          whether to include non-positional sequence features
 +   * @param minmax
 +   * @param summary
-    *          if true, build a shortened summary report (for tooltip)
 +   * @return
 +   */
 +  int createSequenceAnnotationReport(final StringBuilder sb,
            SequenceI sequence, boolean showDbRefs, boolean showNpFeats,
-           Hashtable minmax, boolean summary)
 -          boolean tableWrap, Map<String, float[][]> minmax)
++          Map<String, float[][]> minmax, boolean summary)
    {
      String tmp;
 -    tip.append("<i>");
 +    sb.append("<i>");
  
      int maxWidth = 0;
      if (sequence.getDescription() != null)
      {
        tmp = sequence.getDescription();
 -      tip.append("<br>" + tmp);
 +      sb.append("<br>").append(tmp);
        maxWidth = Math.max(maxWidth, tmp.length());
      }
      SequenceI ds = sequence;
        ds = ds.getDatasetSequence();
      }
      DBRefEntry[] dbrefs = ds.getDBRefs();
-     Arrays.sort(dbrefs, comparator);
      if (showDbRefs && dbrefs != null)
      {
 -      for (int i = 0; i < dbrefs.length; i++)
++      // note this sorts the refs held on the sequence!
++      Arrays.sort(dbrefs, comparator);
 +      boolean ellipsis = false;
++      String source = null;
 +      String lastSource = null;
 +      int countForSource = 0;
++      int sourceCount = 0;
++      boolean moreSources = false;
++      int lineLength = 0;
++
 +      for (DBRefEntry ref : dbrefs)
 +      {
-         String source = ref.getSource();
++        source = ref.getSource();
 +        if (source == null)
 +        {
 +          // shouldn't happen
 +          continue;
 +        }
 +        boolean sourceChanged = !source.equals(lastSource);
 +        if (sourceChanged)
 +        {
++          lineLength = 0;
 +          countForSource = 0;
++          sourceCount++;
++        }
++        if (sourceCount > MAX_SOURCES && summary)
++        {
++          ellipsis = true;
++          moreSources = true;
++          break;
 +        }
 +        lastSource = source;
 +        countForSource++;
 +        if (countForSource == 1 || !summary)
 +        {
 +          sb.append("<br>");
 +        }
-         if (countForSource < 3 || !summary)
++        if (countForSource <= MAX_REFS_PER_SOURCE || !summary)
 +        {
 +          String accessionId = ref.getAccessionId();
-           int len = accessionId.length() + 1;
++          lineLength += accessionId.length() + 1;
 +          if (countForSource > 1 && summary)
 +          {
 +            sb.append(", ").append(accessionId);
-             len++;
++            lineLength++;
 +          }
 +          else
 +          {
 +            sb.append(source).append(" ").append(accessionId);
-             len += source.length();
++            lineLength += source.length();
 +          }
-           maxWidth = Math.max(maxWidth, len);
++          maxWidth = Math.max(maxWidth, lineLength);
 +        }
-         if (countForSource == 3 && summary)
++        if (countForSource == MAX_REFS_PER_SOURCE && summary)
 +        {
-           sb.append(", ...");
++          sb.append(COMMA).append(ELLIPSIS);
 +          ellipsis = true;
 +        }
 +      }
-       if (ellipsis) {
-         sb.append("<br>(Output Sequence Details to list all database references)");
++      if (moreSources)
+       {
 -        tip.append("<br>");
 -        tmp = dbrefs[i].getSource() + " " + dbrefs[i].getAccessionId();
 -        tip.append(tmp);
 -        maxWidth = Math.max(maxWidth, tmp.length());
++        sb.append("<br>").append(ELLIPSIS).append(COMMA).append(source)
++                .append(COMMA).append(ELLIPSIS);
++      }
++      if (ellipsis)
++      {
++        sb.append("<br>(");
++        sb.append(MessageManager.getString("label.output_seq_details"));
++        sb.append(")");
        }
      }
  
--    // ADD NON POSITIONAL SEQUENCE INFO
++    /*
++     * add non-positional features if wanted
++     */
      SequenceFeature[] features = sequence.getSequenceFeatures();
      if (showNpFeats && features != null)
      {
        {
          if (features[i].begin == 0 && features[i].end == 0)
          {
 -          int sz = -tip.length();
 -          List<SequenceFeature> tfeat = new ArrayList<SequenceFeature>();
 -          tfeat.add(features[i]);
 -          appendFeatures(tip, 0, tfeat, minmax);
 -          sz += tip.length();
 +          int sz = -sb.length();
-           List<SequenceFeature> tfeat = Collections
-                   .singletonList(features[i]);
-           appendFeatures(sb, 0, tfeat, minmax);
++          appendFeature(sb, 0, minmax, features[i]);
 +          sz += sb.length();
            maxWidth = Math.max(maxWidth, sz);
          }
        }
      }
++    sb.append("</i>");
 +    return maxWidth;
 +  }
++
++  public void createTooltipAnnotationReport(final StringBuilder tip,
++          SequenceI sequence, boolean showDbRefs, boolean showNpFeats,
++          Map<String, float[][]> minmax)
++  {
++    int maxWidth = createSequenceAnnotationReport(tip, sequence,
++            showDbRefs, showNpFeats, minmax, true);
 -    if (tableWrap && maxWidth > 60)
++    if (maxWidth > 60)
+     {
 -      tip.insert(0, "<table width=350 border=0><tr><td><i>");
 -      tip.append("</i></td></tr></table>");
++      // ? not sure this serves any useful purpose
++      // tip.insert(0, "<table width=350 border=0><tr><td>");
++      // tip.append("</td></tr></table>");
+     }
 -
+   }
  }
index 0000000,a96a2a8..1392157
mode 000000,100644..100644
--- /dev/null
@@@ -1,0 -1,185 +1,185 @@@
+ /*
+  * Jalview - A Sequence Alignment Editor and Viewer ($$Version-Rel$$)
+  * Copyright (C) $$Year-Rel$$ The Jalview Authors
+  * 
+  * This file is part of Jalview.
+  * 
+  * Jalview is free software: you can redistribute it and/or
+  * modify it under the terms of the GNU General Public License 
+  * as published by the Free Software Foundation, either version 3
+  * of the License, or (at your option) any later version.
+  *  
+  * Jalview is distributed in the hope that it will be useful, but 
+  * WITHOUT ANY WARRANTY; without even the implied warranty 
+  * of MERCHANTABILITY or FITNESS FOR A PARTICULAR 
+  * PURPOSE.  See the GNU General Public License for more details.
+  * 
+  * You should have received a copy of the GNU General Public License
+  * along with Jalview.  If not, see <http://www.gnu.org/licenses/>.
+  * The Jalview Authors are detailed in the 'AUTHORS' file.
+  */
+ package jalview.io;
+ import static org.testng.AssertJUnit.assertEquals;
+ import jalview.datamodel.SequenceFeature;
+ import java.util.Hashtable;
+ import java.util.Map;
+ import org.testng.annotations.Test;
+ public class SequenceAnnotationReportTest
+ {
+   @Test(groups = "Functional")
+   public void testAppendFeature_disulfideBond()
+   {
+     SequenceAnnotationReport sar = new SequenceAnnotationReport(null);
 -    StringBuffer sb = new StringBuffer();
++    StringBuilder sb = new StringBuilder();
+     sb.append("123456");
+     SequenceFeature sf = new SequenceFeature("disulfide bond", "desc", 1,
+             3, 1.2f, "group");
+     // residuePos == 2 does not match start or end of feature, nothing done:
+     sar.appendFeature(sb, 2, null, sf);
+     assertEquals("123456", sb.toString());
+     // residuePos == 1 matches start of feature, text appended (but no <br>)
+     // feature score is not included
+     sar.appendFeature(sb, 1, null, sf);
+     assertEquals("123456disulfide bond 1:3", sb.toString());
+     // residuePos == 3 matches end of feature, text appended
+     // <br> is prefixed once sb.length() > 6
+     sar.appendFeature(sb, 3, null, sf);
+     assertEquals("123456disulfide bond 1:3<br>disulfide bond 1:3",
+             sb.toString());
+   }
+   @Test(groups = "Functional")
+   public void testAppendFeature_status()
+   {
+     SequenceAnnotationReport sar = new SequenceAnnotationReport(null);
 -    StringBuffer sb = new StringBuffer();
++    StringBuilder sb = new StringBuilder();
+     SequenceFeature sf = new SequenceFeature("METAL", "Fe2-S", 1, 3,
+             Float.NaN, "group");
+     sf.setStatus("Confirmed");
+     sar.appendFeature(sb, 1, null, sf);
+     assertEquals("METAL 1 3; Fe2-S; (Confirmed)", sb.toString());
+   }
+   @Test(groups = "Functional")
+   public void testAppendFeature_withScore()
+   {
+     SequenceAnnotationReport sar = new SequenceAnnotationReport(null);
 -    StringBuffer sb = new StringBuffer();
++    StringBuilder sb = new StringBuilder();
+     SequenceFeature sf = new SequenceFeature("METAL", "Fe2-S", 1, 3, 1.3f,
+             "group");
+     Map<String, float[][]> minmax = new Hashtable<String, float[][]>();
+     sar.appendFeature(sb, 1, minmax, sf);
+     /*
+      * map has no entry for this feature type - score is not shown:
+      */
+     assertEquals("METAL 1 3; Fe2-S", sb.toString());
+     /*
+      * map has entry for this feature type - score is shown:
+      */
+     minmax.put("METAL", new float[][] { { 0f, 1f }, null });
+     sar.appendFeature(sb, 1, minmax, sf);
+     // <br> is appended to a buffer > 6 in length
+     assertEquals("METAL 1 3; Fe2-S<br>METAL 1 3; Fe2-S Score=1.3",
+             sb.toString());
+     /*
+      * map has min == max for this feature type - score is not shown:
+      */
+     minmax.put("METAL", new float[][] { { 2f, 2f }, null });
+     sb.setLength(0);
+     sar.appendFeature(sb, 1, minmax, sf);
+     assertEquals("METAL 1 3; Fe2-S", sb.toString());
+   }
+   @Test(groups = "Functional")
+   public void testAppendFeature_noScore()
+   {
+     SequenceAnnotationReport sar = new SequenceAnnotationReport(null);
 -    StringBuffer sb = new StringBuffer();
++    StringBuilder sb = new StringBuilder();
+     SequenceFeature sf = new SequenceFeature("METAL", "Fe2-S", 1, 3,
+             Float.NaN, "group");
+     sar.appendFeature(sb, 1, null, sf);
+     assertEquals("METAL 1 3; Fe2-S", sb.toString());
+   }
+   @Test(groups = "Functional")
+   public void testAppendFeature_clinicalSignificance()
+   {
+     SequenceAnnotationReport sar = new SequenceAnnotationReport(null);
 -    StringBuffer sb = new StringBuffer();
++    StringBuilder sb = new StringBuilder();
+     SequenceFeature sf = new SequenceFeature("METAL", "Fe2-S", 1, 3,
+             Float.NaN, "group");
+     sf.setValue("clinical_significance", "Benign");
+     sar.appendFeature(sb, 1, null, sf);
+     assertEquals("METAL 1 3; Fe2-S; Benign", sb.toString());
+   }
+   @Test(groups = "Functional")
+   public void testAppendFeature_withScoreStatusClinicalSignificance()
+   {
+     SequenceAnnotationReport sar = new SequenceAnnotationReport(null);
 -    StringBuffer sb = new StringBuffer();
++    StringBuilder sb = new StringBuilder();
+     SequenceFeature sf = new SequenceFeature("METAL", "Fe2-S", 1, 3, 1.3f,
+             "group");
+     sf.setStatus("Confirmed");
+     sf.setValue("clinical_significance", "Benign");
+     Map<String, float[][]> minmax = new Hashtable<String, float[][]>();
+     minmax.put("METAL", new float[][] { { 0f, 1f }, null });
+     sar.appendFeature(sb, 1, minmax, sf);
+     assertEquals("METAL 1 3; Fe2-S Score=1.3; (Confirmed); Benign",
+             sb.toString());
+   }
+   @Test(groups = "Functional")
+   public void testAppendFeature_DescEqualsType()
+   {
+     SequenceAnnotationReport sar = new SequenceAnnotationReport(null);
 -    StringBuffer sb = new StringBuffer();
++    StringBuilder sb = new StringBuilder();
+     SequenceFeature sf = new SequenceFeature("METAL", "METAL", 1, 3,
+             Float.NaN, "group");
+     // description is not included if it duplicates type:
+     sar.appendFeature(sb, 1, null, sf);
+     assertEquals("METAL 1 3", sb.toString());
+     sb.setLength(0);
+     sf.setDescription("Metal");
+     // test is case-sensitive:
+     sar.appendFeature(sb, 1, null, sf);
+     assertEquals("METAL 1 3; Metal", sb.toString());
+   }
+   @Test(groups = "Functional")
+   public void testAppendFeature_stripHtml()
+   {
+     SequenceAnnotationReport sar = new SequenceAnnotationReport(null);
 -    StringBuffer sb = new StringBuffer();
++    StringBuilder sb = new StringBuilder();
+     SequenceFeature sf = new SequenceFeature("METAL",
+             "<html><body>hello<em>world</em></body></html>", 1, 3,
+             Float.NaN, "group");
+     sar.appendFeature(sb, 1, null, sf);
+     // !! strips off </body> but not <body> ??
+     assertEquals("METAL 1 3; <body>hello<em>world</em>", sb.toString());
+     sb.setLength(0);
+     sf.setDescription("<br>&kHD>6");
+     sar.appendFeature(sb, 1, null, sf);
+     // if no <html> tag, html-encodes > and < (only):
+     assertEquals("METAL 1 3; &lt;br&gt;&kHD&gt;6", sb.toString());
+   }
+ }