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

Mark "has no effect" features as unsupported #3904

Merged
merged 2 commits into from
Apr 18, 2019
Merged

Conversation

Elchi3
Copy link
Member

@Elchi3 Elchi3 commented Apr 16, 2019

These notes make no sense to me. We should just mark them as unsupported imo.

@Elchi3 Elchi3 added the data:css 🎨 Compat data for CSS features. https://developer.mozilla.org/docs/Web/CSS label Apr 16, 2019
@ddbeck
Copy link
Collaborator

ddbeck commented Apr 16, 2019

I think there was a bit of a debate about this when I was originally migrating some CSS data. The meaning of the notes seems significant (though I can't speak to their accuracy). I learned at the time that there's a nuance for valid-but-ineffectual versus invalid-and-ignored. To illustrate the point:

some-property: valid-value;
some-property: recognized-but-ineffectual-value;

Line one gets applied; then line two overrides line one to do something unexpected.

some-property: valid-value;
some-property: invalid-value;

Line one gets applied; line two does nothing.

If we're going to remove this data, I think we should substantiate that the behavior is the latter and not the former before proceeding.

@Elchi3
Copy link
Member Author

Elchi3 commented Apr 16, 2019

Then this situation isn't really partial support, but worse than no support, if I understand it correctly?
In that case, I would probably still mark this as version_added: false, but also have a note that warns about the parsing/recognizing of the value.

@jpmedley
Copy link
Collaborator

jpmedley commented Apr 16, 2019 via email

@queengooborg
Copy link
Collaborator

Agreed. A recognized value that does nothing is an unimplemented value, rather than a partially implemented one.

@Elchi3 Elchi3 changed the title Remove "has no effect" wording Mark "has no effect" features as unsupported Apr 17, 2019
@Elchi3
Copy link
Member Author

Elchi3 commented Apr 17, 2019

Updated this to keep the note but to set version_added to false and remove partial_support.
For Firefox, I removed the ranges this introduced, as I don't think it is that helpful anymore.
Does this make more sense?

@queengooborg
Copy link
Collaborator

Yeah, actually, that makes much more sense, and the note is a lot clearer now. It's looking perfect!

@ExE-Boss
Copy link
Contributor

ExE-Boss commented Apr 17, 2019

Technically, you could argue that it is partially_supported, since the parser recognises it, but the renderer treats it the same as if it was never specified in the whole cascade.

@queengooborg
Copy link
Collaborator

I think that because of the fact that it doesn’t do anything, no real implementation was actually made. Thus, “false” is the more appropriate value.

@ddbeck
Copy link
Collaborator

ddbeck commented Apr 18, 2019

By analogy to Web APIs, I think this would be something like a feature where a method exists but, when you call it, it returns null and doesn't do anything. It's not not implemented, but only just. This is perhaps a corner case for our schema. partial_implementation implies that it satisfies some part of the expected behavior (which is wrong here); "version_added": false implies that this will behave like other unimplemented features (which is wrong here). The note necessarily has to do almost all the work.

partial_implementation is probably more technically correct, but I wonder if that's unhelpful to data consumers. Consider an MDN table: is it better to more aggressively discourage the use of the feature (red box) or to call out to people trying to use and debug the feature (yellow box)? I guess I don't have an answer here, but I don't feel like either option is noticeably worse or better (as long as the notes are preserved, which I do feel strongly about).

@ddbeck
Copy link
Collaborator

ddbeck commented Apr 18, 2019

Which is a long-winded way of saying that I'm +0 on this change; Florian, you can merge it as is if you want, or change it if you've been inspired by this conversation, it's your call. :-)

@Elchi3
Copy link
Member Author

Elchi3 commented Apr 18, 2019

Yes, I think the main point of BCD is to be helpful for web developers who want to use a feature. webplatform-tests, for example and on the contrary, has insight into all details of the implementation of a feature and is technically correct.
However, on BCD, I think, it is an editorial choice whether we call something unsupported or partially supported (and yes partial support almost always goes with notes explaining the situation).

#3273 is interesting for seeing this in the context of APIs. There it is was decided to go with partial_support.

is it better to more aggressively discourage the use of the feature (red box) or to call out to people trying to use and debug the feature (yellow box)?

I think this is a core question here. I'm not sure I know enough about the CSS feature in question here to fully answer it. (same for #3273 – I don't have much familiarity with that API either.)

BTW, the motivation for this change was pragmatic: this had "version_added: true" and how would I come up with a version number for this weirdly supported/unsupported/partially supported feature? So, I thought easiest would be to go with version_added: false.

@ddbeck
Copy link
Collaborator

ddbeck commented Apr 18, 2019

FWIW, I spent some time playing with align-content and start and end values. Their behavior in Chrome is surprising. The feedback from the browser is more along the lines of you're doing this wrong than this doesn't work, which is misleading. But you also can't do anything materially useful with them.

the motivation for this change was pragmatic

This is a fair point. I'm fine with leaving this as false and leaving the partial_implementation on other features like this, where we have version numbers. I don't think we need a consistent rule for edge cases like this, unless the rule is: we should strive to be as informative as we can be with the data we can substantiate.

How about this as a compromise: merge this change as is, but open an issue inviting anyone who wants to to do to the research to find out when it was partially implemented. If we ever get that version number, we can switch to partial_implementation, but absent that data, leave this as false.

@Elchi3
Copy link
Member Author

Elchi3 commented Apr 18, 2019

Sounds good to me. I'm merging this then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:css 🎨 Compat data for CSS features. https://developer.mozilla.org/docs/Web/CSS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants