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.
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
script: Add pre element obsolete width attribute support #31792
Conversation
This should also affect layout, right? I think you are missing the part that actually implements the behavior that this attribute affects. |
Nope the spec says the prop only needs to reflect the attr. Also Firefox, Chrome and Safari don't adjust the width of the element when this attr is set. |
🔨 Triggering try run (#8363529805) for Linux WPT |
Test results for linux-wpt-layout-2020 from try job (#8363529805): Flaky unexpected result (15)
Stable unexpected results that are known to be intermittent (17)
Stable unexpected results (2)
|
|
Looks like there is one new test passing and one new pass.
|
I fixed the failing test by implementing |
🔨 Triggering try run (#8366515763) for Linux WPT |
Test results for linux-wpt-layout-2020 from try job (#8366515763): Flaky unexpected result (22)
Stable unexpected results that are known to be intermittent (13)
Stable unexpected results (2)
|
|
@mrobinson wpt passed but it expects that some tests fail, do I need to update that fail and remove the expected fails? |
You will need to regenerate results for these newly passing tests:
and then also for legacy layout
|
Pinging @Loirooriol to ask his opinion about whether or not it makes sense to implement support for a deprecated attribute. I have my doubts in this case because it doesn't really do anything. @bplaat This is a well-made change, but perhaps it makes sense to work on something that's actually functional? I'm happy to help guide you to something if you are interested. |
Sure this is quite useless but is in the spec so it is worth implementing. I choose it because it is quite small and it would make for a nice first pr |
So this is https://html.spec.whatwg.org/multipage/obsolete.html#HTMLPreElement-partial, which is in "16.3 Requirements for implementations", so it seems good. |
@bplaat Sounds like this is fine! Please update test results and then we can land this. |
@bplaat There is a test still failing for legacy layout. Just run the commands above, but also the legacy layout version:
|
@mrobinson I run the command for both layouts and committed the files, when I run the commands again no files are changed |
This pr implements the
pre
element obsolete width attribute, it's quite a small change but also my first in the servo project 🎉./mach build -d
does not report any errors./mach test-tidy
does not report any errorsI can't find a wpt test that tests the this obsolete attribute, maybe I should add one?