New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #36544. #36578

Merged
merged 1 commit into from Oct 20, 2017

Conversation

Projects
None yet
2 participants
@brandonbloom
Contributor

brandonbloom commented Oct 19, 2017

This was definitely a bug in my "Go Last" implementation, but it was only exposed by using 499086d (and/also 273dbee) which made more aggressive use of the addOrReplaceInStack method.

The problem is that when the navigation stack is full, the array is shifted over and indexes are patched up. This can result with a lastIndex that exceeds the length of the array, since setIndex was being called twice during the patch up with inconsistent array lengths. The fix is to only call setIndex once, after the length is corrected. This also fixes a bug in "Go Last" where the "last" position is wrong if it's not this.stack.length - 1 after the history buffer fills up. To fix that, we slide the lastIndex over along with the array unshift. If the last index falls off the left side of the array, it will be -1, which is already handled by last() to fallback to the behavior of back().

@bpasero bpasero added this to the October 2017 milestone Oct 20, 2017

@bpasero bpasero merged commit 8ce940f into Microsoft:master Oct 20, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla All CLA requirements met.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment