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.

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

[cssom] Test the camelcased attributes are implemented as attributes. #16900

Merged

Conversation

@emilio
Copy link
Contributor

emilio commented May 17, 2019

WebKit and Blink do not implement these correctly, and thus fail the last bit of
the test.

@wpt-pr-bot wpt-pr-bot requested review from dbaron, lilles and plinss May 17, 2019
@emilio emilio requested a review from annevk May 17, 2019
…plemented as attributes.

WebKit and Blink do not implement these correctly, and thus fail the last bit of
the test.
@emilio emilio force-pushed the emilio:csstyledeclaration-attributes branch from 8edf88c to a94364e May 17, 2019
@emilio emilio changed the title [cssom] Test the camelcased and webkit-cased attributes are indeed implemented as attributes. [cssom] Test the camelcased attributes are implemented as attributes. May 17, 2019
@gsnedders
Copy link
Contributor

gsnedders commented May 20, 2019

Alternatively, do we just want to add an idlharness.js test for:

partial interface CSSStyleDeclaration {
  [CEReactions] attribute [TreatNullAs=EmptyString] CSSOMString _camel_cased_attribute;
};

partial interface CSSStyleDeclaration {
  [CEReactions] attribute [TreatNullAs=EmptyString] CSSOMString _webkit_cased_attribute;
};

partial interface CSSStyleDeclaration {
  [CEReactions] attribute [TreatNullAs=EmptyString] CSSOMString _dashed_attribute;
};

and assume that all properties defined in CSS2 are supported CSS properties? (Plus, I guess, some -webkit- property?)

That would get us coverage of not just this but all the things that idlharness.js asserts about attributes.

I think really ideally we'd use the data in Reffy to generate those partial interface definitions as part of the IDL defined in each spec that defines CSS properties, hmm. @foolip?

@annevk
annevk approved these changes May 20, 2019
Copy link
Member

annevk left a comment

This looks good, but it's indeed weird that CSSOM is not IDL-tested already. @foolip @lukebjerring?

@emilio emilio merged commit 5a62d84 into web-platform-tests:master May 20, 2019
10 checks passed
10 checks passed
wpt.fyi - chrome[experimental] Chrome results
Details
wpt.fyi - safari[experimental] Safari results
Details
Azure Pipelines Build #20190517.51 succeeded
Details
Azure Pipelines (./wpt test-jobs) ./wpt test-jobs succeeded
Details
Azure Pipelines (affected tests (Safari Technology Preview)) affected tests (Safari Technology Preview) succeeded
Details
Azure Pipelines (affected tests without changes (Safari Technology Preview)) affected tests without changes (Safari Technology Preview) succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests) wpt.fyi hook: safari-preview-affected-tests succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests-without-changes) wpt.fyi hook: safari-preview-affected-tests-without-changes succeeded
Details
Taskcluster (pull_request) TaskGroup: success
Details
wpt.fyi - firefox[experimental] Firefox results
Details
@emilio emilio deleted the emilio:csstyledeclaration-attributes branch May 20, 2019
@emilio
Copy link
Contributor Author

emilio commented May 20, 2019

I just sent a proposal to parse the CSS specs for properties and generate syntax tests (https://lists.w3.org/Archives/Public/www-style/2019May/0013.html), which I think it could be used to know which attributes to test, if that's desirable.

@gsnedders
Copy link
Contributor

gsnedders commented May 20, 2019

@annevk it does have idlharness.js tested, but there's a bunch of IDL definitions where there are attributes defined for each supported CSS property (and CSSOM doesn't enumerate them), and we don't currently create an attribute for every CSS property defined in any spec.

@annevk
Copy link
Member

annevk commented May 20, 2019

Ah right, it's defined in prose these days? That would indeed create this issue. 😟

marcoscaceres added a commit that referenced this pull request Jul 23, 2019
…plemented as attributes. (#16900)

WebKit and Blink do not implement these correctly, and thus fail the last bit of
the test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.