ladybird
ladybird copied to clipboard
LibWeb/CSS: Use default font metrics when computing `:root` font size
Prevents repeatedly building font size when using rem in the :root element.
Fixes #139 and #339. Resetera font is still larger than it should be, but it no longer continuously grows until crash.
Not sure if https://github.com/dzfrias/ladybird/blob/568de76fd26c1f5349132834e236a43f3cd5cfbc/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp#L2218 is also relevant here. I'm not sure why absolutize_values doesn't mess everything up since it's also using root font metrics... regardless, it didn't cause any issues.
Not sure if dzfrias/ladybird@
568de76/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp#L2218 is also relevant here. I'm not sure whyabsolutize_valuesdoesn't mess everything up since it's also using root font metrics... regardless, it didn't cause any issues.
The way it works is a bit messy unfortunately. I think this also needs updating, and might be why the font is still larger than it should be, though I don't know why it's not continuously growing.
I'm having a little bit of trouble since create_document_style also uses absolutize_values, and I'm not sure what the correct behavior should be. I'm not fully sure how the font selection stuffs works.
Is there a certain reference or algorithm that these methods of StyleComputer are following? Is it all from the CSS spec?
I don't think there's a spec algorithm. Most of the CSS specs just tell you what the end result should be and you have to figure it out yourself.
The relevant spec would be https://www.w3.org/TR/css-fonts-4/ - I don't think any others will affect this.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!
The original issue is still there on master, so it should be cool to continue with this PR. @dzfrias is this PR still blocked by "the way we do fonts right now"?
It's mostly blocked because I'm hesitant to try to merge something that I don't think is a particularly good solution to this problem. My solution is more of a patch over the problem, and I don't understand the CSS computation code well enough to know if the changes I'm making are appropriate.
That said, I haven't read the relevant code in a while now, so things might've changed.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!
This pull request has been closed because it has not had recent activity. Feel free to re-open if you wish to still contribute these changes. Thank you for your contributions!