hyperHTML icon indicating copy to clipboard operation
hyperHTML copied to clipboard

select attr is "reset" with FF

Open albertosantini opened this issue 7 years ago • 12 comments

In FF (57+) the selected item toggles between the correct one, bar, and the first one foo.

With Chrome (62.x), it works as expected: bar is always selected (and displayed).

I read again the documentation about boolean attrs. :) Am I missing anything here?

class Foo {
    static init() {
        const render = hyperHTML.bind(document.querySelector("foo"));

        const state = {
            items: [
                { id: 1, text: "foo" },
                { id: 2, text: "bar" },
                { id: 3, text: "baz" }
            ],
            selectedItem: 2
        };

        render`
            <select>${
                state.items.map(item => hyperHTML.wire()`
                    <option value="${item.id}" selected="${state.selectedItem === item.id}">
                        ${item.text}
                    </option>`)
            }</select>
        `;
    }
}

setInterval(Foo.init, 100);

Live example: https://plnkr.co/edit/evFbsDnoN7iwooJpqw33?p=preview

albertosantini avatar Nov 28 '17 12:11 albertosantini

this is the most horrifying Firefox bug I've ever seen to date: https://codepen.io/WebReflection/pen/WXaeWj?editors=0010

WebReflection avatar Nov 28 '17 14:11 WebReflection

Filed a bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1421260

WebReflection avatar Nov 28 '17 14:11 WebReflection

Argh! Will get it prioritized and fixed.

marcoscaceres avatar Nov 28 '17 15:11 marcoscaceres

I saw the bug has been confirmed. I tested with FF 56-58.beta and it is there: so I suppose it is also in the previous releases. Hope it will be fixed for the final 58.

In the meantime my ugly workaround is the following one:

class Foo {
    static init() {
        const render = hyperHTML.bind(document.querySelector("foo"));

        const state = {
            items: [
                { id: 1, text: "foo" },
                { id: 2, text: "bar" },
                { id: 3, text: "baz" }
            ],
            selectedItem: 2
        };

        const node = render`
            <select>${
                state.items.map(item => hyperHTML.wire()`
                    <option value="${item.id}">
                        ${item.text}
                    </option>
                `)
            }</select>
        `;
        document.querySelector(`option[value='${state.selectedItem}']`).selected = true;
    }
}

setInterval(Foo.init, 100);

Live example: https://plnkr.co/edit/6Guxg7nEIYPzJmt5IaZR?p=preview

Tested also in my complex use case and it works correctly.

albertosantini avatar Nov 29 '17 10:11 albertosantini

it's a diffing algorithm issue, if you use majinbuu engine instead, everything is fine without needing workarounds.

https://plnkr.co/edit/IgjYDmRx2doychS6zDnw?p=preview

WebReflection avatar Nov 29 '17 10:11 WebReflection

Bug resolved in FF 58.0b8. :)

P.S. No, I am sorry, I see ghosts. It is not resolved. Sorry for the noise.

albertosantini avatar Dec 04 '17 09:12 albertosantini

I am planning to implement domdiff as universal solution for hyperHTML so that current engine will go and majinbuu won't be needed.

That should also, hopefully, solve, this issue forever, even on older FF.

WebReflection avatar Dec 04 '17 15:12 WebReflection

actually ... never mind: https://codepen.io/WebReflection/pen/EbMwwV?editors=0010

this bug affects Vue.js and Preact with keyed elements too ... this is very bad

WebReflection avatar Dec 04 '17 15:12 WebReflection

P.S. @albertosantini if you used wires properly thought, you wouldn't have experienced any of this. You are trashing options every time. It's still a FF bug, but you should never trash nodes when it's possible.

WebReflection avatar Dec 04 '17 15:12 WebReflection

I am planning to implement domdiff as universal solution for hyperHTML so that current engine will go and majinbuu won't be needed.

What is the reason behind this decision?

ematipico avatar Dec 04 '17 16:12 ematipico

What is the reason behind this decision?

nobody is writing hyperHTML engines and AFAIK nobody uses hyperhtml-majinbuu but the point is that current default engine doesn't scale in terms of performance and neither does majinbuu.

Instead of having all these hybrid, not fully usable solutions for this or that case, I'd rather use what Vue.js ~~and Preact~~ are using to diff their content (a fork of snabbdom) which apparently performs as good as majinbuu but it scales way more without needing checks on array size and all the caveats one or the other solution has.

On top of that, removing complexity and abstractions with 3rd parts engines, will reduce the library size closer to 4K instead of 5K

WebReflection avatar Dec 04 '17 16:12 WebReflection

in case anyone is curious about engine-less edition of hyperHTML: https://github.com/WebReflection/hyperHTML/pull/149

WebReflection avatar Dec 04 '17 17:12 WebReflection