Stop generating raw #target anchor tags

These don't work well on MSIE 6, where we need to use History.newItem
to ensure that the history system works correctly.

Bug: GERRIT-86
Signed-off-by: Shawn O. Pearce <sop@google.com>

src/main/java/com/google/gerrit/client/changes/PatchSetPanel.java

@@ -16,7 +16,6 @@ package com.google.gerrit.client.changes;
 
 import com.google.gerrit.client.FormatUtil;
 import com.google.gerrit.client.Gerrit;
-import com.google.gerrit.client.Link;
 import com.google.gerrit.client.SignOutEvent;
 import com.google.gerrit.client.SignOutHandler;
 import com.google.gerrit.client.data.ApprovalType;
@@ -32,6 +31,7 @@ import com.google.gerrit.client.reviewdb.Project;
 import com.google.gerrit.client.reviewdb.UserIdentity;
 import com.google.gerrit.client.rpc.Common;
 import com.google.gerrit.client.rpc.GerritCallback;
+import com.google.gerrit.client.ui.AccountDashboardLink;
 import com.google.gerrit.client.ui.RefreshListener;
 import com.google.gwt.event.dom.client.ClickEvent;
 import com.google.gwt.event.dom.client.ClickHandler;
@@ -45,11 +45,10 @@ import com.google.gwt.user.client.ui.Composite;
 import com.google.gwt.user.client.ui.DisclosurePanel;
 import com.google.gwt.user.client.ui.FlowPanel;
 import com.google.gwt.user.client.ui.Grid;
+import com.google.gwt.user.client.ui.InlineLabel;
 import com.google.gwt.user.client.ui.Panel;
 import com.google.gwt.user.client.ui.HTMLTable.CellFormatter;
 import com.google.gwtexpui.clippy.client.CopyableLabel;
-import com.google.gwtexpui.safehtml.client.SafeHtml;
-import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder;
 import com.google.gwtjsonrpc.client.VoidResult;
 
 import java.util.ArrayList;
@@ -236,37 +235,25 @@ class PatchSetPanel extends Composite implements OpenHandler<DisclosurePanel> {
       return;
     }
 
-    final SafeHtmlBuilder m = new SafeHtmlBuilder();
-
+    final FlowPanel fp = new FlowPanel();
+    fp.setStyleName("gerrit-PatchSetUserIdentity");
     if (who.getName() != null) {
       final Account.Id aId = who.getAccount();
       if (aId != null) {
-        m.openAnchor();
-        m.setAttribute("href", "#" + Link.toAccountDashboard(aId));
-      }
-      m.append(who.getName());
-      if (aId != null) {
-        m.closeAnchor();
+        fp.add(new AccountDashboardLink(who.getName(), aId));
+      } else {
+        final InlineLabel lbl = new InlineLabel(who.getName());
+        lbl.setStyleName("gerrit-AccountName");
+        fp.add(lbl);
       }
     }
-
     if (who.getEmail() != null) {
-      if (m.hasContent()) {
-        m.append(' ');
-      }
-      m.append('<');
-      m.append(who.getEmail());
-      m.append('>');
+      fp.add(new InlineLabel("<" + who.getEmail() + ">"));
     }
-
     if (who.getDate() != null) {
-      if (m.hasContent()) {
-        m.append(' ');
-      }
-      m.append(FormatUtil.mediumFormat(who.getDate()));
+      fp.add(new InlineLabel(FormatUtil.mediumFormat(who.getDate())));
     }
-
-    SafeHtml.set(infoTable, row, 1, m);
+    infoTable.setWidget(row, 1, fp);
   }
 
   private void populateActions(final PatchSetDetail detail) {

src/main/java/com/google/gerrit/client/changes/PatchTable.java

@@ -18,6 +18,7 @@ import com.google.gerrit.client.Link;
 import com.google.gerrit.client.reviewdb.Patch;
 import com.google.gerrit.client.reviewdb.PatchSet;
 import com.google.gerrit.client.ui.NavigationTable;
+import com.google.gerrit.client.ui.PatchLink;
 import com.google.gwt.core.client.GWT;
 import com.google.gwt.event.dom.client.ClickEvent;
 import com.google.gwt.event.dom.client.ClickHandler;
@@ -27,6 +28,8 @@ import com.google.gwt.user.client.History;
 import com.google.gwt.user.client.IncrementalCommand;
 import com.google.gwt.user.client.ui.Composite;
 import com.google.gwt.user.client.ui.FlowPanel;
+import com.google.gwt.user.client.ui.Label;
+import com.google.gwt.user.client.ui.Widget;
 import com.google.gwt.user.client.ui.HTMLTable.Cell;
 import com.google.gwtexpui.progress.client.ProgressBar;
 import com.google.gwtexpui.safehtml.client.SafeHtml;
@@ -85,9 +88,45 @@ public class PatchTable extends Composite {
       super.resetHtml(html);
     }
 
-    @Override
-    public void setRowItem(final int row, final Patch p) {
-      super.setRowItem(row, p);
+    void initializeRow(final int row, final Patch p) {
+      final int C_PATH = 2;
+      final int C_SIDEBYSIDE = 4;
+      setRowItem(row, p);
+
+      Widget nameCol;
+      if (p.getPatchType() == Patch.PatchType.UNIFIED) {
+        nameCol = new PatchLink.SideBySide(p.getFileName(), p.getKey());
+      } else {
+        nameCol = new PatchLink.Unified(p.getFileName(), p.getKey());
+      }
+      if (p.getSourceFileName() != null) {
+        final String text;
+        if (p.getChangeType() == Patch.ChangeType.RENAMED) {
+          text = Util.M.renamedFrom(p.getSourceFileName());
+        } else if (p.getChangeType() == Patch.ChangeType.COPIED) {
+          text = Util.M.copiedFrom(p.getSourceFileName());
+        } else {
+          text = Util.M.otherFrom(p.getSourceFileName());
+        }
+        final Label line = new Label(text);
+        line.setStyleName("SourceFilePath");
+        final FlowPanel cell = new FlowPanel();
+        cell.add(nameCol);
+        cell.add(line);
+        nameCol = cell;
+      }
+      table.setWidget(row, C_PATH, nameCol);
+
+      int C_UNIFIED = C_SIDEBYSIDE + 1;
+      if (p.getPatchType() == Patch.PatchType.UNIFIED) {
+        table.setWidget(row, C_SIDEBYSIDE, new PatchLink.SideBySide(Util.C
+            .patchTableDiffSideBySide(), p.getKey()));
+
+      } else if (p.getPatchType() == Patch.PatchType.BINARY) {
+        C_UNIFIED = C_SIDEBYSIDE + 2;
+      }
+      table.setWidget(row, C_UNIFIED, new PatchLink.Unified(Util.C
+          .patchTableDiffUnified(), p.getKey()));
     }
 
     void appendHeader(final SafeHtmlBuilder m) {
@@ -140,33 +179,6 @@ public class PatchTable extends Composite {
       m.openTd();
       m.addStyleName(S_DATA_CELL);
       m.addStyleName("FilePathCell");
-
-      m.openAnchor();
-      if (p.getPatchType() == Patch.PatchType.UNIFIED) {
-        m.setAttribute("href", "#" + Link.toPatchSideBySide(p.getKey()));
-      } else {
-        m.setAttribute("href", "#" + Link.toPatchUnified(p.getKey()));
-      }
-      m.append(p.getFileName());
-      m.closeAnchor();
-
-      if (p.getSourceFileName() != null) {
-        final String secondLine;
-        if (p.getChangeType() == Patch.ChangeType.RENAMED) {
-          secondLine = Util.M.renamedFrom(p.getSourceFileName());
-
-        } else if (p.getChangeType() == Patch.ChangeType.COPIED) {
-          secondLine = Util.M.copiedFrom(p.getSourceFileName());
-
-        } else {
-          secondLine = Util.M.otherFrom(p.getSourceFileName());
-        }
-        m.br();
-        m.openSpan();
-        m.setStyleName("SourceFilePath");
-        m.append(secondLine);
-        m.closeSpan();
-      }
       m.closeTd();
 
       m.openTd();
@@ -189,9 +201,7 @@ public class PatchTable extends Composite {
       switch (p.getPatchType()) {
         case UNIFIED:
           openlink(m, 2);
-          m.setAttribute("href", "#" + Link.toPatchSideBySide(p.getKey()));
-          m.append(Util.C.patchTableDiffSideBySide());
-          closelink(m);
+          m.closeTd();
           break;
 
         case BINARY: {
@@ -201,6 +211,7 @@ public class PatchTable extends Composite {
             case DELETED:
             case MODIFIED:
               openlink(m, 1);
+              m.openAnchor();
               m.setAttribute("href", base + "^1");
               m.append(Util.C.patchTableDownloadPreImage());
               closelink(m);
@@ -213,6 +224,7 @@ public class PatchTable extends Composite {
             case MODIFIED:
             case ADDED:
               openlink(m, 1);
+              m.openAnchor();
               m.setAttribute("href", base + "^0");
               m.append(Util.C.patchTableDownloadPostImage());
               closelink(m);
@@ -230,9 +242,7 @@ public class PatchTable extends Composite {
       }
 
       openlink(m, 1);
-      m.setAttribute("href", "#" + Link.toPatchUnified(p.getKey()));
-      m.append(Util.C.patchTableDiffUnified());
-      closelink(m);
+      m.closeTd();
 
       m.closeTr();
     }
@@ -242,7 +252,6 @@ public class PatchTable extends Composite {
       m.addStyleName(S_DATA_CELL);
       m.addStyleName("DiffLinkCell");
       m.setAttribute("colspan", colspan);
-      m.openAnchor();
     }
 
     private void closelink(final SafeHtmlBuilder m) {
@@ -320,7 +329,7 @@ public class PatchTable extends Composite {
 
         case 1:
           while (row < list.size()) {
-            table.setRowItem(row + 1, list.get(row));
+            table.initializeRow(row + 1, list.get(row));
             if ((++row % 50) == 0 && longRunning()) {
               updateMeter();
               return true;

src/main/java/com/google/gerrit/client/ui/AccountDashboardLink.java

@@ -30,21 +30,25 @@ public class AccountDashboardLink extends DirectScreenLink {
     return ai != null ? new AccountDashboardLink(ai) : null;
   }
 
-  private AccountInfo account;
+  private Account.Id accountId;
 
   public AccountDashboardLink(final AccountInfo ai) {
     this(FormatUtil.name(ai), ai);
   }
 
   public AccountDashboardLink(final String text, final AccountInfo ai) {
+    this(text, ai.getId());
+    setTitle(FormatUtil.nameEmail(ai));
+  }
+
+  public AccountDashboardLink(final String text, final Account.Id ai) {
     super(text, Link.toAccountDashboard(ai));
     addStyleName("gerrit-AccountName");
-    setTitle(FormatUtil.nameEmail(ai));
-    account = ai;
+    accountId = ai;
   }
 
   @Override
   protected Screen createScreen() {
-    return new AccountDashboardScreen(account.getId());
+    return new AccountDashboardScreen(accountId);
   }
 }

src/main/java/com/google/gerrit/public/gerrit.css

@@ -645,6 +645,15 @@
 .gerrit-PatchSetInfoBlock {
   margin-bottom: 10px;
 }
+.gerrit-PatchSetUserIdentity {
+  white-space: nowrap;
+}
+.gerrit-PatchSetUserIdentity .gerrit-AccountName {
+  display: inline
+}
+.gerrit-PatchSetUserIdentity .gwt-InlineLabel {
+  margin-left: 0.2em;
+}
 
 .gerrit-PatchSetActions {
   margin-bottom: 10px;