Merge "Use a utility function for reloading the window"
diff --git a/java/com/google/gerrit/server/change/RebaseChangeOp.java b/java/com/google/gerrit/server/change/RebaseChangeOp.java
index a548262..b43996e 100644
--- a/java/com/google/gerrit/server/change/RebaseChangeOp.java
+++ b/java/com/google/gerrit/server/change/RebaseChangeOp.java
@@ -219,8 +219,13 @@
.setFireRevisionCreated(fireRevisionCreated)
.setCheckAddPatchSetPermission(checkAddPatchSetPermission)
.setValidate(validate)
- .setSendEmail(sendEmail)
- .setWorkInProgress(!rebasedCommit.getFilesWithGitConflicts().isEmpty());
+ .setSendEmail(sendEmail);
+
+ if (!rebasedCommit.getFilesWithGitConflicts().isEmpty()
+ && !notes.getChange().isWorkInProgress()) {
+ patchSetInserter.setWorkInProgress(true);
+ }
+
if (postMessage) {
patchSetInserter.setMessage(
messageForRebasedChange(rebasedPatchSetId, originalPatchSet.id(), rebasedCommit));
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 1c7b54b..50b7a7c 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -1360,6 +1360,39 @@
}
@Test
+ public void rebaseDoesNotAddWorkInProgress() throws Exception {
+ PushOneCommit.Result r = createChange();
+
+ // create an unrelated change so that we can rebase
+ testRepo.reset("HEAD~1");
+ PushOneCommit.Result unrelated = createChange();
+ gApi.changes().id(unrelated.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(unrelated.getChangeId()).current().submit();
+
+ gApi.changes().id(r.getChangeId()).rebase();
+
+ // change is still ready for review after rebase
+ assertThat(gApi.changes().id(r.getChangeId()).get().workInProgress).isNull();
+ }
+
+ @Test
+ public void rebaseDoesNotRemoveWorkInProgress() throws Exception {
+ PushOneCommit.Result r = createChange();
+ change(r).setWorkInProgress();
+
+ // create an unrelated change so that we can rebase
+ testRepo.reset("HEAD~1");
+ PushOneCommit.Result unrelated = createChange();
+ gApi.changes().id(unrelated.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(unrelated.getChangeId()).current().submit();
+
+ gApi.changes().id(r.getChangeId()).rebase();
+
+ // change is still work in progress after rebase
+ assertThat(gApi.changes().id(r.getChangeId()).get().workInProgress).isTrue();
+ }
+
+ @Test
public void rebaseConflict_conflictsAllowed() throws Exception {
String patchSetSubject = "patch set change";
String patchSetContent = "patch set content";
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java b/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
index 9e944a2..7e6a822 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
@@ -495,6 +495,24 @@
}
@Test
+ public void rebaseDoesNotAddToAttentionSet() throws Exception {
+ PushOneCommit.Result r = createChange();
+ change(r).setWorkInProgress();
+ change(r).addReviewer(user.email());
+
+ // create an unrelated change so that we can rebase
+ testRepo.reset("HEAD~1");
+ PushOneCommit.Result unrelated = createChange();
+ gApi.changes().id(unrelated.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(unrelated.getChangeId()).current().submit();
+
+ gApi.changes().id(r.getChangeId()).rebase();
+
+ // rebase has no impact on the attention set
+ assertThat(r.getChange().attentionSet()).isEmpty();
+ }
+
+ @Test
public void readyForReviewWhileRemovingReviewerRemovesThemToAttentionSet() throws Exception {
PushOneCommit.Result r = createChange();
change(r).setWorkInProgress();
diff --git a/polygerrit-ui/app/api/diff.ts b/polygerrit-ui/app/api/diff.ts
index 7e619c2..5d7125c 100644
--- a/polygerrit-ui/app/api/diff.ts
+++ b/polygerrit-ui/app/api/diff.ts
@@ -225,6 +225,9 @@
code_range: LineRange;
}
+/** LOST LineNumber is for ported comments without a range, they have their own
+ * line number and are added on top of the FILE row in gr-diff
+ */
export declare type LineNumber = number | 'FILE' | 'LOST';
/** The detail of the 'create-comment' event dispatched by gr-diff. */
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
index 9fb5e74..f6f80b3 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
@@ -67,6 +67,7 @@
import * as shadow from 'shadow-selection-polyfill/shadow.js';
import {CreateCommentEventDetail as CreateCommentEventDetailApi} from '../../../api/diff';
+import {isSafari} from '../../../utils/dom-util';
const NO_NEWLINE_BASE = 'No newline at end of base file.';
const NO_NEWLINE_REVISION = 'No newline at end of revision file.';
@@ -355,11 +356,13 @@
_getShadowOrDocumentSelection() {
// When using native shadow DOM, the selection returned by
// document.getSelection() cannot reference the actual DOM elements making
- // up the diff, because they are in the shadow DOM of the gr-diff element.
- // This takes the shadow DOM selection if one exists.
+ // up the diff in Safari because they are in the shadow DOM of the gr-diff
+ // element. This takes the shadow DOM selection if one exists.
return this.root instanceof ShadowRoot && this.root.getSelection
? this.root.getSelection()
- : shadow.getRange(this.root);
+ : isSafari()
+ ? shadow.getRange(this.root)
+ : document.getSelection();
}
_observeNodes() {
diff --git a/polygerrit-ui/app/utils/dom-util.ts b/polygerrit-ui/app/utils/dom-util.ts
index 2023da0..aa83173 100644
--- a/polygerrit-ui/app/utils/dom-util.ts
+++ b/polygerrit-ui/app/utils/dom-util.ts
@@ -250,3 +250,11 @@
}
return root.activeElement as HTMLElement;
}
+
+// Whether the browser is Safari. Used for polyfilling unique browser behavior.
+export function isSafari() {
+ return (
+ /^((?!chrome|android).)*safari/i.test(navigator.userAgent) ||
+ (/iPad|iPhone|iPod/.test(navigator.userAgent) && !window.MSStream)
+ );
+}
