Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix PageView cache re-insertions. #4936

Merged
merged 1 commit into from

2 participants

Nicholas Nethercote Yury Delendik
Nicholas Nethercote

If a PageView is re-inserted into the cache, there's a splice() call done which I'm pretty sure is supposed to just remove the existing copy. But in Firefox a.splice(n) has a special behaviour -- it causes every element after the nth to be removed. (See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/splice for details. I don't know what it does in other browsers.) This causes pages to be removed from the cache without having destroy() called on them.

This patch changes it to a.splice(n, 1), which always removes just the single element.

(BTW, there are two splice(i) calls in src/shared/util.js that may also be buggy.)

Yury Delendik

Trivial. Thanks

Yury Delendik yurydelendik merged commit 69d7227 into from
Nicholas Nethercote nnethercote deleted the branch
Nicholas Nethercote

Yury, do you think the splice(i) calls in src/shared/util.js are buggy?

Yury Delendik

do you think the splice(i) calls in src/shared/util.js are buggy?

Maybe, but there are not that important -- there are related to the unhandled rejections in the Promise polyfill.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 13, 2014
  1. Nicholas Nethercote
This page is out of date. Refresh to see the latest.
Showing with 1 addition and 1 deletion.
  1. +1 −1  web/ui_utils.js
2  web/ui_utils.js
View
@@ -248,7 +248,7 @@ var Cache = function cacheCache(size) {
this.push = function cachePush(view) {
var i = data.indexOf(view);
if (i >= 0) {
- data.splice(i);
+ data.splice(i, 1);
}
data.push(view);
if (data.length > size) {
Something went wrong with that request. Please try again.