Skip to content

experiment: tell stylo to use width/height attrs#393

Draft
AustinMReppert wants to merge 1 commit intoDioxusLabs:mainfrom
AustinMReppert:feature/width-attribute
Draft

experiment: tell stylo to use width/height attrs#393
AustinMReppert wants to merge 1 commit intoDioxusLabs:mainfrom
AustinMReppert:feature/width-attribute

Conversation

@AustinMReppert
Copy link
Copy Markdown
Contributor

This was a quick experiment to see what happens when making stylo use width/height attrs. In general it seems more correct, however there are some regressions especially with replaced content. Some of the regressions should not have passed to begin with, but there are a few genuine regressions.

AI use:
I used gemini to figure out where to make the change.

WPT changes:
PASS: css/css2/backgrounds/background-001.xht
PASS: css/css2/backgrounds/background-002.xht
PASS: css/css2/backgrounds/background-003.xht
PASS: css/css2/backgrounds/background-006.xht
PASS: css/css2/backgrounds/background-007.xht
PASS: css/css2/backgrounds/background-008.xht
PASS: css/css2/backgrounds/background-009.xht
PASS: css/css2/backgrounds/background-010.xht
PASS: css/css2/backgrounds/background-014.xht
PASS: css/css2/backgrounds/background-018.xht
PASS: css/css2/backgrounds/background-022.xht
PASS: css/css2/backgrounds/background-087.xht
PASS: css/css2/backgrounds/background-182.xht
PASS: css/css2/backgrounds/background-328.xht
PASS: css/css2/backgrounds/background-329.xht
FAIL: css/css2/backgrounds/background-attachment-applies-to-007.xht
FAIL: css/css2/backgrounds/background-attachment-applies-to-009.xht
FAIL: css/css2/backgrounds/background-attachment-applies-to-012.xht
FAIL: css/css2/backgrounds/background-attachment-applies-to-013.xht
FAIL: css/css2/backgrounds/background-attachment-applies-to-014.xht
PASS: css/css2/backgrounds/background-image-cover-002.xht
PASS: css/css2/backgrounds/background-image-cover-004.xht
PASS: css/css2/backgrounds/background-image-cover-attachment-001.xht
PASS: css/css2/backgrounds/background-image-transparency-001.xht
PASS: css/css2/backgrounds/background-repeat-001.xht
PASS: css/css2/backgrounds/background-repeat-002.xht
PASS: css/css2/backgrounds/background-repeat-003.xht
PASS: css/css2/backgrounds/background-repeat-005.xht
PASS: css/css2/floats-clear/float-replaced-width-011.xht
PASS: css/css2/inline-svg-intrinsic-size-100-percent-1.html
PASS: css/css2/normal-flow/block-replaced-width-006.xht
PASS: css/css2/normal-flow/inline-block-replaced-width-006.xht
PASS: css/css2/normal-flow/inline-replaced-width-006.xht
PASS: css/css2/positioning/absolute-replaced-width-006.xht
PASS: css/css2/positioning/absolute-replaced-width-013.xht
PASS: css/css2/positioning/absolute-replaced-width-020.xht
PASS: css/css2/positioning/absolute-replaced-width-027.xht
PASS: css/css2/positioning/absolute-replaced-width-034.xht
PASS: css/css2/positioning/absolute-replaced-width-069.xht
PASS: css/css2/positioning/absolute-replaced-width-076.xht
PASS: css/css-flexbox/svg-no-natural-size-grandchild.html
FAIL: css/css-grid/alignment/replaced-alignment-with-aspect-ratio-003.tentative.html
FAIL: css/css-sizing/aspect-ratio/replaced-element-037.html
PASS: css/css-sizing/svg-intrinsic-size-005.html
FAIL: css/css-transforms/perspective-svg-001.html
PASS: css/css-transforms/transforms-rotate-degree-45.html
PASS: css/css-viewport/zoom/svg-path-simple.html
PASS: css/css-viewport/zoom/svg-path.html
PASS: css/css-viewport/zoom/svg-transform.html
PASS: css/css-viewport/zoom/svg-viewbox.html

Comment on lines +915 to 923
push_style(PropertyDeclaration::Width(Size::LengthPercentage(
NonNegative(width),
)));
}
} else if *name == local_name!("width") {
if let Some(width) = parse_size_attr(value, |_| true) {
use style::values::generics::{NonNegative, length::Size};

push_style(PropertyDeclaration::Width(Size::LengthPercentage(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is duplicating the block above it! We should definitely dedupe. We should also consider following the linked spec https://html.spec.whatwg.org/multipage/rendering.html#dimRendering

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants