diff --git a/src/DiffEngineViewer.Tests/PreviousChangeTests.cs b/src/DiffEngineViewer.Tests/PreviousChangeTests.cs
new file mode 100644
index 00000000..f8c5ef43
--- /dev/null
+++ b/src/DiffEngineViewer.Tests/PreviousChangeTests.cs
@@ -0,0 +1,64 @@
+///
+/// Stepping back through the changed blocks of the entry on screen. Forty rows with changes at 3,
+/// 17 and 33 - so rows 2, 16 and 32 - and sixteen body rows to show them in.
+///
+public class PreviousChangeTests
+{
+ ///
+ /// A block ending on the row immediately above the viewport is one the reader has not been
+ /// taken to, and it used to be stepped over as though it were the block they were already in.
+ ///
+ [Test]
+ public async Task Lands_on_a_block_ending_just_above_the_viewport()
+ {
+ var state = At(17);
+
+ var moved = ViewerSession.Apply(state, CommandKind.PreviousChange);
+
+ await Assert.That(moved.ScrollTop).IsEqualTo(16);
+ }
+
+ ///
+ /// The same thing with nothing above it to fall through to, where the result was no movement
+ /// at all rather than the wrong movement.
+ ///
+ [Test]
+ public async Task Reaches_the_first_block_from_the_row_below_it()
+ {
+ var state = At(3);
+
+ var moved = ViewerSession.Apply(state, CommandKind.PreviousChange);
+
+ await Assert.That(moved.ScrollTop).IsEqualTo(2);
+ }
+
+ ///
+ /// A viewport that really is inside a block still steps off it, or previous would never leave
+ /// the block it is in.
+ ///
+ [Test]
+ public async Task Steps_off_the_block_the_viewport_is_in()
+ {
+ var state = At(16);
+
+ var moved = ViewerSession.Apply(state, CommandKind.PreviousChange);
+
+ await Assert.That(moved.ScrollTop).IsEqualTo(2);
+ }
+
+ [Test]
+ public async Task Stays_at_the_first_block()
+ {
+ var state = At(2);
+
+ var moved = ViewerSession.Apply(state, CommandKind.PreviousChange);
+
+ await Assert.That(moved.ScrollTop).IsEqualTo(2);
+ }
+
+ static SessionState At(int scrollTop) =>
+ Fixtures.File(Fixtures.Long(true), Fixtures.Long(false)) with
+ {
+ ScrollTop = scrollTop
+ };
+}
diff --git a/src/DiffEngineViewer/ViewerSession.cs b/src/DiffEngineViewer/ViewerSession.cs
index d59969e7..daf70d93 100644
--- a/src/DiffEngineViewer/ViewerSession.cs
+++ b/src/DiffEngineViewer/ViewerSession.cs
@@ -1039,7 +1039,13 @@ static int NextChange(IReadOnlyList rows, int from)
static int PreviousChange(IReadOnlyList rows, int from)
{
- var index = Math.Min(from, rows.Count) - 1;
+ // The top row of the viewport, not the one above it. Stepping off from there took the
+ // block ending immediately above the viewport for the block the viewport was in, and
+ // skipped past it to the one before - or, with nothing before it, refused to move at all.
+ var index = Math.Min(from, rows.Count - 1);
+
+ // So step off only when the viewport really is sitting in a block, which is when its top
+ // row is itself a change.
while (index >= 0 &&
IsChange(rows[index]))
{