Skip to content
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.

Sign up for GitHub

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

Jump to bottom

v2.2.10 Breaks CSS text in happy-dom/vitest? #2989

Closed
1 task done
thequailman opened this issue Nov 11, 2024 · 5 comments · Fixed by #2991
Closed
1 task done

v2.2.10 Breaks CSS text in happy-dom/vitest? #2989

thequailman opened this issue Nov 11, 2024 · 5 comments · Fixed by #2991
Assignees
@dead-claudia
Labels
Area: Core For anything dealing with Mithril core itself Status: Needs reproduction For issues that lack a demo or example that reproduces the unexpected behavior in question Type: Bug For bugs and any other unexpected breakage

Comments

@thequailman
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Mithril.js Version

2.2.10

Browser and OS

Vitest

Project

No response

Code

Checking the value of `style.cssText` for any element returns an empty string as of 2.2.10.  Prior to this (2.2.9), the value showed the actual CSS text.  This was immensely helpful for testing CSS values with vitest/happy-dom.

Steps to Reproduce

Using vitest or happy-dom, getElement and check the style.cssText value.

Expected Behavior

It should return the CSS values, I think.

Observed Behavior

Empty string is always returned.

Context

Breaks a bunch of my tests.
@thequailman thequailman added the Area: Core For anything dealing with Mithril core itself label Nov 11, 2024
@dead-claudia
Copy link
Member

Could you provide code to reproduce your error? Your "Code" section just contains error info, which isn't the purpose of the section.

@dead-claudia dead-claudia added Type: Bug For bugs and any other unexpected breakage Status: Needs reproduction For issues that lack a demo or example that reproduces the unexpected behavior in question labels Nov 11, 2024
@thequailman
Copy link
Author

It's a bit complicated as you need to have vitest and happy dom setup, something like this might work:

import { GlobalRegistrator } from "@happy-dom/global-registrator";
import m from "mithril";

GlobalRegistrator.register();
m.mount(document.body, {
  view: () => {
    return m("button", {
      style: {
        color: "blue",
      },
    }
  }
})
console.log(document.querySelector("button").style.cssText) // should return "color: blue"

@dead-claudia
Copy link
Member

@thequailman Could you slap together a repo with instructions?

Was hoping for something self-contained.

@kfule
Copy link
Contributor

kfule commented Nov 15, 2024

It seems that happy-dom does not support the setting to style dashed-properties unlike other real browsers and jsdom.
With #2985 , the settings to the dashed-properties are now used more aggressively.

happy-dom example (can not set via dashed-property)

// happy-dom
const { Window } = require('happy-dom');
const window = new Window();

window.document.body.innerHTML = '<p>Hello world</p>';
const element = window.document.querySelector("p");
console.log(element.textContent) // Hello world

// not supported (against spec.)
element.style["font-size"] = "10px"

// supported
// element.style.fontSize = "10px"
// element.style["fontSize"] = "10px"
// element.style.setProperty("font-size","10px")

// unintended support (cssText is "fontSize: 10px;")
// element.style.setProperty("fontSize","10px")

console.log(element.style.cssText); // no prints

jsdom example (can set via dashed-property)

// jsdom
const jsdom = require("jsdom");
const { JSDOM } = jsdom;

const dom = new JSDOM(`<!DOCTYPE html><p>Hello world</p>`);
const element = dom.window.document.querySelector("p");
console.log(element.textContent) // Hello world

// supported
element.style["font-size"] = "10px"
// element.style.fontSize = "10px"
// element.style["fontSize"] = "10px"
// element.style.setProperty("font-size","10px")

// not supported (No problem, as it behaves according to the spec.)
// element.style.setProperty("fontSize","10px")

console.log(element.style.cssText); // prints "font-size: 10px;"

@dead-claudia It might be better to change to a variation using includes() or indexOf(), as the old proposal in #2985.

kfule added a commit to kfule/mithril.js that referenced this issue Nov 15, 2024
@kfule
updateStyle(): use setProperty() when css vars and dashed-properties, f…
36158fe 
…ixes MithrilJS#2989
@kfule kfule mentioned this issue Nov 15, 2024
Merged
8 tasks
@dead-claudia dead-claudia mentioned this issue Nov 15, 2024
Open
@dead-claudia
Copy link
Member

Reported that happy-dom example in capricorn86/happy-dom#1613 as they claim to (be trying to) implement a full headless web browser. I'm still open to a fix for this, but do note that our internal mocks are not supported for general use (and haven't been for a long time).

dead-claudia pushed a commit to kfule/mithril.js that referenced this issue Nov 18, 2024
@kfule @dead-claudia
updateStyle(): use setProperty() when css vars and dashed-properties, f…
abcbacc 
…ixes MithrilJS#2989
@JAForbes JAForbes mentioned this issue Nov 18, 2024
Merged
dead-claudia pushed a commit that referenced this issue Nov 25, 2024
@kfule @dead-claudia
updateStyle(): use setProperty() when css vars and dashed-properties, f…
e10ab83 
…ixes #2989 (#2991)
@dead-claudia dead-claudia mentioned this issue Dec 7, 2024
Open
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dead-claudia dead-claudia

Labels
Area: Core For anything dealing with Mithril core itself Status: Needs reproduction For issues that lack a demo or example that reproduces the unexpected behavior in question Type: Bug For bugs and any other unexpected breakage
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

updateStyle(): use setProperty() when css vars and dashed-properties, fixes #2989 kfule/mithril.js
3 participants
@dead-claudia @thequailman @kfule