LadybirdBrowser
Icons do not load properly in "usermap.serenityos.org"
Summary
When I look at https://usermap.serenityos.org/, the "https://unpkg.com/[email protected]/dist/images/marker-icon.png" doesn't show up in ladybird.
Operating system
macOS
Steps to reproduce
- load website https://usermap.serenityos.org/
- see icon
Expected behavior
shows icon properly
Actual behavior
dose not show icon
URL for a reduced test case
/
HTML/SVG/etc. source for a reduced test case
/
Log output and (if possible) backtrace
/
Screenshots or screen recordings
Build flags or config settings
No response
Contribute a patch?
- [ ] I’ll contribute a patch for this myself.
I think Ladybird's getComputedStyle() doesn't properly resolve relative URLs in CSS to absolute URLs. It doesn't take into account origin of CSS file. Here's the minimal repro from what this website is doing:
<head>
<link
rel="stylesheet"
href="https://unpkg.com/[email protected]/dist/leaflet.css"
/>
</head>
<body></body>
<script>
function repro() {
var testDiv = document.createElement("div");
testDiv.className = "leaflet-default-icon-path";
document.body.appendChild(testDiv);
// This className has background-image: url(images/marker-icon.png);
var computedStyle = window.getComputedStyle(testDiv, null);
var backgroundImage = computedStyle.backgroundImage;
console.log("getComputedStyle returns:", backgroundImage);
}
repro();
</script>
Chrome: getComputedStyle returns: url("https://unpkg.com/[email protected]/dist/images/marker-icon.png")
Ladybird: "getComputedStyle returns:" "url("images/marker-icon.png")"
Fixed here: https://github.com/trflynn89/ladybird/commit/5dfde2533bc7b2b8f1bf9447e63de7c327504a75
But I'm not familiar with this area. @AtkinsSJ does the above patch make sense?
Fixed here: trflynn89@5dfde25
But I'm not familiar with this area. @AtkinsSJ does the above patch make sense?
It's very unclear when we're supposed to take relative URLs and absolutize them. The algorithms I implemented tell us that they should remain relative until we fetch their resources, but clearly that's not true. Maybe I should raise a spec issue about this - I thought I already had but I can't see one.
For computed style, there are kind of two options... one is as you've done in that patch, but the other is to actually replace the URL with an absolute one when we compute the style, which would be ImageStyleValue::absolutized() etc. I'm inclined to think that's the better option, but I wouldn't object to your patch getting merged.
