435

Update at 2018.10.31

This bug has been fixed in iOS 12.1, have a good day~

I found a problem with Array's value state in the newly released iOS 12 Safari, for example, code like this:

<!DOCTYPE html>
<html>
<head>
    <meta charset="utf-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=0">
    <title>iOS 12 Safari bugs</title>
    <script type="text/javascript">
    window.addEventListener("load", function ()
    {
        let arr = [1, 2, 3, 4, 5];
        alert(arr.join());

        document.querySelector("button").addEventListener("click", function ()
        {
            arr.reverse();
        });
    });
    </script>
</head>
<body>
    <button>Array.reverse()</button>
    <p style="color:red;">test: click button and refresh page, code:</p>
</body>
</html>

After refreshing the page, the array's value is still reversed. Is this a bug or a feature of new Safari?

Here is a demo page. Try to use it with iOS 12 Safari: https://abelyao.github.io/others/ios12-safari-bug.html

Community
  • 1
  • 1
abelyao
  • 1,571
  • 2
  • 7
  • 7

4 Answers4

276

It's definitely a BUG! And it's a very serious bug.

The bug is due to the optimization of array initializers in which all values are primitive literals. For example, given the function:

function buildArray() {
    return [1, null, 'x'];
}

All returned array references from calls to buildArray() will link to the same memory, and some methods such as toString() will have their results cached. Normally, to preserve consistency, any mutable operation on such optimized arrays will copy the data to a separate memory space and link to it; this pattern is called copy-on-write, or CoW for short.

The reverse() method mutates the array, so it should trigger a copy-on-write. But it doesn't, because the original implementor (Keith Miller of Apple) missed the reverse() case, even though he had written many testcases.

This bug was reported to Apple on August 21. The fix landed in the WebKit repository on August 27 and shipped in Safari 12.0.1 and iOS 12.1 on October 30, 2018.

MultiplyByZer0
  • 6,302
  • 3
  • 32
  • 48
hax
  • 1,446
  • 1
  • 10
  • 13
  • 12
    Note: Safari 12.0 on Mac OS X also have the same issue. – hax Sep 19 '18 at 11:48
  • 18
    Yeah it’s been fixed in the sources already, and shipped in Safari Technology Preview already. Try https://cdn.miss.cat/demo/ios12-safari-bug.html in Safari Technology Preview 65. You’ll find that it doesn’t have the bug. – sideshowbarker Sep 19 '18 at 12:24
  • 7
    I don’t believe the underlying cause of the bug is the result of an index mixup; instead, it seems to be caused by neglecting to check whether an object is immutable before modifying it. The slice issue may have a similar explanation, but it’s not the same but and won’t be fixed by the patch for reverse, as far as I can tell. You should consider opening a WebKit bug report for the slice issue. – Zenexer Sep 19 '18 at 14:46
  • 6
    @Zenexer You are right. I wrote this answer before I found the https://bugs.webkit.org/show_bug.cgi?id=188794 and see the source code. I will edit my answer. – hax Sep 19 '18 at 14:54
76

I wrote a lib to fix the bug. https://www.npmjs.com/package/array-reverse-polyfill

This is the code:

(function() {
  function buggy() {
    var a = [1, 2];
    return String(a) === String(a.reverse());
  }
  if(!buggy()) return;
  var r = Array.prototype.reverse;
  Array.prototype.reverse = function reverse() {
    if (Array.isArray(this)) this.length = this.length;
    return r.call(this);
  }
})();
Community
  • 1
  • 1
Edire Fan
  • 681
  • 4
  • 7
  • 4
    Update at any time. Welcome to contribute. – Edire Fan Sep 19 '18 at 11:52
  • 15
    @zephi, I guess that writing on length (`this.length = this.length`) will trigger Copy On Write, so will change the memory address of the array, and so will fix the behavior of `reverse`. – Cœur Sep 20 '18 at 01:47
14

This is a bug in webkit. Though this has been solved at their end but not yet shipped with iOS GM release. One of the solutions to this problem:

(function() {
  function getReverseStr() {
    return [1, 2].reverse();
  }

  var n1 = getReverseStr()[0];
  var n2 = getReverseStr()[0];
  // check if there is an issue
  if(n1 != n2) {
    var origReverseFunction = Array.prototype.reverse;
    Array.prototype.reverse = function() {
      var newArr = this.slice();
      // use original reverse function so that edge cases are taken care of
      origReverseFunction.apply(newArr, arguments);
      var that = this;
      // copy reversed array
      newArr.forEach(function(value, index) {
        that[index] = value;
      });
      return this;
    }
  }
})();
jsist
  • 5,223
  • 3
  • 28
  • 43
6

It seems not to be cached if the number of elements changes.
I was able to avoid this like this.

<!DOCTYPE html>
<html>
<head>
    <meta charset="utf-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=0">
    <title>iOS 12 Safari bugs</title>
    <script type="text/javascript">
    window.addEventListener("load", function ()
    {
        let arr = [1, 2, 3, 4, 5];
        arr.push('');
        arr.pop();
        alert(arr.join());

        document.querySelector("button").addEventListener("click", function ()
        {
            arr.reverse();
        });
    });
    </script>
</head>
<body>
    <button>Array.reverse()</button>
    <p style="color:red;">test: click button and refresh page, code:</p>
</body>
</html>
Atsushi Sasaki
  • 77
  • 1