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

Data structure: __compat and features on the same level #283

Closed
Elchi3 opened this issue Jul 7, 2017 · 5 comments
Closed

Data structure: __compat and features on the same level #283

Elchi3 opened this issue Jul 7, 2017 · 5 comments
Labels
schema ⚙️ Isses or pull requests regarding the JSON schema files used in this project.

Comments

@Elchi3
Copy link
Member

Elchi3 commented Jul 7, 2017

Right now, we were differentiating between "feature tables" and "feature aggregate tables" in the KS macro [1]. This requires that features and __compat never appear on the same level. However, Jean-Yves points out that you actually want to have this oftentimes.
Like you would have __compat, href and target on the same level in this structure, where __compat indicates the compat for the base element itself:

(1) Proposed structure

  • data
    • html
      • elements
        • base
          • __compat
            • basic_support
          • href
            • __compat
              • basic_support
              • relative_uris
          • target
            • __compat
              • basic_support

For APIs, we ran into the same problem, when we want to indicate support for the interface itself. I ended up repeating the interface name in these cases as a workaround to avoid __compat on the same level as the other features.

(2) Current API structure

  • data
    • javascript
      • builtins
        • String
          • String
            • __compat
              • basic_support
          • replace
            • __compat
              • basic_support

(3) Current API structure applied to the HTML base element structure

  • data
    • html
      • elements
        • base
          • base
            • __compat
              • basic_support
          • href
            • __compat
              • basic_support
              • relative_uris
          • target
            • __compat
              • basic_support

The String.String or base.base structure might not be ideal here and so we need to decide if we want to rewrite the {{compat}} macro to work with structures that allow __compat and feature trees on the same level. Basically, if we want structure (1) instead of structure (2) and (3).

Thoughts?

[1] https://github.com/mozilla/kumascript/blob/master/macros/Compat.ejs#L424

@jwhitlock
Copy link
Contributor

👍 to adding compat information for base elements. I would handle it by splitting {{compat}} into {{compat}} and {{compatAggregate}}, rather than assuming intent from the data.

@wbamberg
Copy link
Collaborator

wbamberg commented Jul 7, 2017

I agree with John that having different macros for aggregate and non-aggregate is good, especially as the way the tables are presented is different. That involves rewriting macros and updating hundreds of WebExtension pages, but it's a thing we should do anyway.

But I think it's important to think not just in the abstract, but in terms of how we want to present compat data to people.

In your (1), for example, I guess this is for https://developer.mozilla.org/en-US/docs/Web/HTML/Element/base. In this scenario, why not use subfeatures for href and target? Put it this way: would you ever expect to have a standalone "feature table" for the target attribute alone? Does the target attribute make sense on its own, rather than as an attribute of base?

I guess the reason you want target to be a "feature" is that it has its own things, that might themselves need fine-grained compat information (_self, _blank and so on). But how do you want to present this to people? What do you want the table for base to look like, and how do you want that table to present compat data for target? Do you really want to have a separate table for target?

In your (2), I think of the String>String feature as describing support for the constructor, not "for the interface itself". For example, it gives you a place to describe different ways the constructor could be called. In the case of String, it's not obvious to me what support "for the interface itself" means, other than support for the "features" - methods, properties, etc - that that interface defines. Again, put another way, is it meaningful to say that String is supported, but none of its features are?

@Elchi3
Copy link
Member Author

Elchi3 commented Jul 10, 2017

I agree that "assuming intent from the data" isn't something we should do. So, yes, generally lets have data structures that make sense rather than what pleases a particular data displayer like MDN at the moment.

The more data we migrate, the more cases of how we want to display the data on MDN appear of course. So we are having the displaying discussion at the same time.

I guess the reason you want target to be a "feature" is that it has its own things, that might themselves need fine-grained compat information

Yes, exactly, but how to display this nested structure of "HTML element -> element attribute -> attribute values" in the MDN compat tables has not been defined and is an MDN HTML reference page design issue rather than a data issue. We need to look into how to present HTML compat data best to our readers there. In the data, we need to make sure to have structures that represent the fine-grained compat data in a meaningful way. I think how it's done in structure (1) makes sense to me.

In your (2), I think of the String>String feature as describing support for the constructor, not "for the interface itself".

Oh yes, you are right. String>String is actually the constructor.

it's not obvious to me what support "for the interface itself" means

Good point. In WebGL there are a few WebGL Extension interfaces, that, if present, enable the extension functionality. The interface can be passed into a method to do that. Maybe this is an edge case, though.

If I want to use an API like String (or say Promise) the "interface itself" might give me an answer if it is present. I might not be interested in getting into the details or sub features to test support, but I just want a general answer if I can use it to some extent. Also the "interface itself" might be prefixed, or use an alternate name, which need to be somehow on this level, too. (Promise was Future in versions X-Y).

If I look at this from a data displayer perspective, I can think of displayers that are interested in more higher level data, for example https://platform-status.mozilla.org/#promise.

Further comments on this welcome, but I think we agree that structure (1) should be allowed (and currently is allowed by the schema). Give me a 👍 when you agree to this, so we can close this issue.

@wbamberg
Copy link
Collaborator

I don't think you need my +1 to close this.

I just worry that we're not always clear on the semantics of the schema, and that by allowing different ways of expressing the same kind of thing "just in case", we'll end up with inconsistencies between different data sets, and data that's confusing to readers. To me, in this case, target looks like a subfeature. I've actually recently handled a similar case by treating search_provider.alternate_urls as a subfeature of chrome_settings_overrides rather than building an additional search_provider feature underneath it: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/manifest.json/chrome_settings_overrides#Browser_compatibility. Was that the right thing to do? Is this just a personal choice thing, and are we then OK with inconsistencies like this?

If we allow features and compat to appear on the same level, what are subfeatures for anyway? Why not just use features for everything?

Yes, exactly, but how to display this nested structure ... is an MDN HTML reference page design issue rather than a data issue.

Yes, but... MDN pages are our only real customer (and therefore source of real requirements) right now. Designing for speculative requirements seems risky to me.

I might not be interested in getting into the details or sub features to test support, but I just want a general answer if I can use it to some extent.

And the answer might be: it's supported "to some extent" if some of its features are supported. What I mean is, "support for the interface" isn't a separate concept from "support for its features": it would be meaningless to say "we support interface XYZ, but you can't actually use it for anything".

@Elchi3
Copy link
Member Author

Elchi3 commented Jul 13, 2017

Was that the right thing to do? Is this just a personal choice thing, and are we then OK with inconsistencies like this?

I think I would have liked having additional search_provider features underneath somehow.

If we allow features and compat to appear on the same level, what are subfeatures for anyway? Why not just use features for everything?

This is an interesting thought. I think it is not always clear what are features and what are subfeatures plus having basic_support for some but not for others. And then a problem of how to display all this in a meaningful way.

Could it look like this? Would it also eliminate basic_support?

CSS

  • data.css.properties
    • background-color (the property)
      • __compat
        • support
        • status
      • alpha_ch_for_hex (a characteristic or value of the property)
        • __compat
          • support
          • status

JS or APIs

  • data.javascript.builtins
    • String (the interface)
      • __compat
        • support
        • status
      • String (the constructor)
        • __compat
          • support
          • status
      • localeCompare (a method)
        • __compat
          • support
          • status
        • locales (method parameter)
          • __compat
            • support
            • status
        • options (method parameter)
          • __compat
            • support
            • status

WebExtension manifest

  • data.webextensions.manifest
    • chrome_settings_overrides (the key)
      • __compat
        • support
        • status
      • homepage (a property)
        • __compat
          • support
          • status
      • search_provider (an object property)
        • __compat
          • support
          • status
        • alternate_urls
          • __compat
            • support
            • status

HTML

  • data.html.elements
    • link (the element)
      • __compat
        • support
        • status
      • charset (a simple attribute)
        • __compat
          • support
          • status
      • rel (a complex attribute)
        • __compat
          • support
          • status
        • prerender (a value of rel)
          • __compat
            • support
            • status

Macro thoughts

The {{compat}} macro might query this as it does now, e.g. {{compat("html.elements.link")}} and a second parameter would indicate how deep it should go into the feature trees. Defaulting to maybe one level. So, it would list the link element and its attributes. If called with {{compat("html.elements.link", 2)}} it would also go into the rel values. If you just want the rel values, you would call {{compat("html.elements.link.rel")}}.

Elchi3 added a commit to mdn/kumascript that referenced this issue Aug 24, 2017
* Change {{compat}} to work with new data structure
For more details see mdn/browser-compat-data#304
and mdn/browser-compat-data#283

* Make WebExtensions use {{compat}} macro

* Address review comments

- remove text coloring (will be redone in the redesign anyway)
- Update depth parameter comment
- change hideNotes to showNotes
- Create a map of browsers
- Change writeTableHead, writeTable and writeSupportCells to use a map of browsers and browserPlatformType
- Change getVersionString back to just return a string
- Add a function getSupportClass that returns a proper CSS class for a cell
- Implement labels that represent the hierarchies that are lost due to flattening for the 2D table

* Address more review comments

* re-introduce "Basic support"
* introduce aggregateMode param (instead of showNotes)
* remove links from WebExt modules headlines

* Add a shim for WebExtBrowserCompat

* Indicate partial support by using a flag on main features

* Use aggregate tables for main Extension API pages
@Elchi3 Elchi3 closed this as completed Aug 25, 2017
@queengooborg queengooborg added the schema ⚙️ Isses or pull requests regarding the JSON schema files used in this project. label Aug 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
schema ⚙️ Isses or pull requests regarding the JSON schema files used in this project.
Projects
None yet
Development

No branches or pull requests

4 participants