codefrau / Smalltalk78

A Smalltalk-78 VM in Javascript
https://codefrau.github.io/Smalltalk78/
49 stars 8 forks source link

Image saving broken in Firefox #2

Open ghost opened 6 years ago

ghost commented 6 years ago

Hi,

why is the lively web version:

  1. incredible slow, I never had any webbrowser which was powerful enough on any of my machines to be able to open this thing in there and it worked. If it's not fixable then please remove the link to the lively web system.

  2. broken, when it finally decides to load and show something then it's an error

  3. Differs from rendering. On your repository the index returns a much more satisfactory result when rendered. The lively web version is just a bunch of boxes, not the expected smalltalk78 workspace.

Please, please don't annihilate your efforts of resurrecting Smalltalk-78. Thank you.

codefrau commented 6 years ago

What browser are you using? It works fine for me in Chrome. I added that to the README.

ghost commented 6 years ago

I'm using Firefox Quantum 57.0.1

And here is how the lively kernel version looks like, I don't think that's how it's supposed to be:

smalltalk78_lively

codefrau commented 6 years ago

As I wrote, it's fine in Chrome. It's likely my fault that it's not working in Firefox, but I have no time to investigate at the moment.

The actual error happens while parsing the image:

RangeError: invalid data view length
stack:nextBytes@https://lively-web.org/users/bert/St78/vm.js:1202:57
readFromBuffer@https://lively-web.org/users/bert/St78/vm.js:1814:31
oopmapFromBuffer@https://lively-web.org/users/bert/St78/vm.js:1227:23
readFromBuffer@https://lively-web.org/users/bert/St78/vm.js:1140:22

My guess is that TypedArrays behave slightly differently.

The Lively version provides a much richer and useful environment for me as a developer than the tools built into the browser. If you don't see the benefits, just don't use it. I coded the VM to not depend on Lively.

In any case I would welcome help in both making the non-lively version richer, and making the lively version more compatible.

codefrau commented 6 years ago

I took a look, and it appears to be a bug in Firefox related to localStorage. It works the first time you visit the Lively page. The image is saved to the browser's localStorage, and loading it works fine.

However, after restarting the browser, the contents of localStorage is not the same as it was. Apparently it gets modified by the roundtrip to Firefox's database. That corrupts the image and we get an error when loading.

The proper fix would be to not rely on localStorage being binary-safe, but use some other storage mechanism like IndexedDB. This requires a greater change.

codefrau commented 6 years ago

That Firefox bug appears to be open for 4 years without action: https://bugzilla.mozilla.org/show_bug.cgi?id=995910

ghost commented 6 years ago

Thanks for looking into the issue. Interesting that Mozilla didn't bother fixing that.

codefrau commented 6 years ago

I'm leaving this issue open because we really need a workaround. A simple solution would be to change the localStorage strings from 16 bit to 8 bit. This will use up twice the space, but that's better than being broken.

The better solution would be to rewrite it, either using IndexedDB directly or using a library like BrowserFS. These operate asynchronously, however. This will result in some more VM complexity.

codefrau commented 6 years ago

Here's a simple test for the problem (also at http://freudenbergs.de/bert/bugs/localstorage.html). It appears to work fine on Chrome and Safari, but fails after restart on Firefox:

 <script>
    const existing = localStorage.persistanceTest;

    if (!existing) {
        let chars = [];
        for (let i = 0; i <= 0xFFFF; i++) {
            chars.push(String.fromCharCode(i));
        }
        localStorage.persistanceTest = chars.join('');
        document.write('Writing to local storage <br>\n');
    }

    document.write('Reading from local storage<br>\n');
    let wrong = 0;
    for (let i = 0; i <= 0xFFFF; i++) {
        const value = existing.charCodeAt(i);
        if (value !== i) {
            wrong++;
            document.write(`${i.toString(16)} changed to ${value.toString(16)}<br>\n`);
        }
    }

    if (wrong) {
        document.write(`${wrong} values were wrong`);
    } else {
        document.write('All values correct');
    }
</script>