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

Touch Events term "expose legacy touch event APIs" not included #560

Closed
zcorpan opened this issue Apr 13, 2022 · 14 comments
Closed

Touch Events term "expose legacy touch event APIs" not included #560

zcorpan opened this issue Apr 13, 2022 · 14 comments

Comments

@zcorpan
Copy link
Member

zcorpan commented Apr 13, 2022

w3c/touch-events#113 was merged yesterday, I had hoped it would fix the build error in whatwg/dom#1071 but it seems not. The term "expose legacy touch event APIs" not included in webref. Per https://respec.org/docs/#data-export I think <dfn class="export"> is the correct incarnation.

https://github.com/w3c/webref/blob/main/ed/dfns/touch-events.json was updated 5 days ago, but the latest reffy run was run more recently.

What's wrong?

@zcorpan
Copy link
Member Author

zcorpan commented Apr 13, 2022

I guess filing this issue fixed it, because now it's there: https://github.com/w3c/webref/blob/main/ed/dfns/touch-events.json#L1266

The build still fails, but maybe it needs some more time for the terms to flow through to the bikeshed build server.

@zcorpan
Copy link
Member Author

zcorpan commented Apr 13, 2022

Is it possible to shorten the time it takes for terms to flow through?

@tidoust
Copy link
Member

tidoust commented Apr 13, 2022

I guess filing this issue fixed it, because now it's there: https://github.com/w3c/webref/blob/main/ed/dfns/touch-events.json#L1266

It's here but note access is private because the definition lacks a data-export attribute, needed for other specs to reference it.

The build still fails, but maybe it needs some more time for the terms to flow through to the bikeshed build server.

As far as I know, Bikeshed does not use webref data yet, so there's not much we can do from a webref perspective...

Is it possible to shorten the time it takes for terms to flow through?

That's being tracked in #486 but see above, that probably won't help you in this case...

@tidoust
Copy link
Member

tidoust commented Apr 13, 2022

It's here but note access is private because the definition lacks a data-export attribute, needed for other specs to reference it.

I responded quickly in-between calls, but now that I had a bit of time to look into it, it seems that the data-export attribute is missing because the spec references a now obsolete version of Respec, which does not seem to process the class="export" that you have in the source.

Edit: I prepared w3c/touch-events#114 to switch the spec to the right profile. I confirm that the term would then correctly be flagged as "exported".

tidoust added a commit to tidoust/touch-events that referenced this issue Apr 13, 2022
The spec referenced an outdated profile of ReSpec. Some editing features were
not supported as a result.

Also see: w3c/webref#560 (comment)
@zcorpan
Copy link
Member Author

zcorpan commented Apr 14, 2022

Thanks @tidoust!

@zcorpan
Copy link
Member Author

zcorpan commented Apr 14, 2022

@tabatkins what is needed for bikeshed to pick up the new term?

@tidoust
Copy link
Member

tidoust commented Apr 14, 2022

@tabatkins what is needed for bikeshed to pick up the new term?

I'll let Tab correct me but unless something changed in Bikeshed recently, adding the spec to the list of specs that Bikeshed knows about is not going to be enough. Bikeshed won't see definitions in this spec because the spec is generated on-the-fly when it is loaded (that's how ReSpec works).

A possible workaround is to rather publish the generated version of the spec under https://w3c.github.io/touch-events/ and not the source file. This can be done with the spec-prod GitHub action. I'd be happy to prepare another PR against the w3c/touch-events repository to add that generation step if you think that's useful (the job would run whenever a commit is pushed to the default branch).

Another option to tell Bikeshed about these definitions is to add them directly as custom definitions in the DOM spec. The spec already contains a custom definition for the TouchEvent interface for instance.

@tabatkins
Copy link
Member

tabatkins commented Apr 14, 2022

With ReSpec specs, it depends on how the markup is done. So long as the dfn contract is satisfied in the source code, Bikeshed can pick it up.

In this case, as it's a plain term definition, everything's already golden (all the defaults are correct) except for the export, and even if it's not exported in the source that can be fixed by adding it to link-defaults in the spec that's using it; there's no need for a custom definition. (There almost never is; most custom definitions are cargo-culted.)

The other thing needed is for the spec to be in Shepherd, which it currently isn't; I've just added it, so we'll just need to wait for the bot to cycle, which usually takes an hour or so.

@zcorpan
Copy link
Member Author

zcorpan commented Apr 14, 2022

Thanks for the help!

The definition uses <dfn class="export">, as per ReSpec documentation, but unlike Bikeshed's dfn contract (which expects data-export=""). Can ReSpec specs use data-export="" instead, currently?

@tabatkins
Copy link
Member

I don't know what class=export does, except possibly put the data-export attribute on it in the processed document?

@zcorpan
Copy link
Member Author

zcorpan commented Apr 14, 2022

Using Inspect Element for https://w3c.github.io/touch-events/#conditionally-exposing-legacy-touch-event-apis it's still class="export". But definitions in IDL have data-export attributes. cc @marcoscaceres why is there a mix? Should class="export" be changed to data-export=""?

@tidoust
Copy link
Member

tidoust commented Apr 14, 2022

Using Inspect Element for https://w3c.github.io/touch-events/#conditionally-exposing-legacy-touch-event-apis it's still class="export". But definitions in IDL have data-export attributes. cc @marcoscaceres why is there a mix? Should class="export" be changed to data-export=""?

See previous comment. The PR I prepared needs to be merged: w3c/touch-events#114

Once that is done, ReSpec will produce a data-export="" attribute in the generated spec.

You may put data-export="" directly in the source as well. ReSpec will transparently copy that over to the generated spec.

zcorpan added a commit to zcorpan/touch-events that referenced this issue Apr 27, 2022
patrickhlauke pushed a commit to w3c/touch-events that referenced this issue Apr 27, 2022
patrickhlauke pushed a commit to w3c/touch-events that referenced this issue Apr 27, 2022
The spec referenced an outdated profile of ReSpec. Some editing features were
not supported as a result.

Also see: w3c/webref#560 (comment)
@tidoust
Copy link
Member

tidoust commented Apr 27, 2022

Closing this issue following spec update earlier today. Definition is now correctly flagged as "public" in Webref data and available in ReSpec's xref database.

Availability in Bikeshed is a different matter and either requires the spec's generation workflow, the spec's source, or integrating Webref data in Bikeshed.

@tidoust tidoust closed this as completed Apr 27, 2022
@zcorpan
Copy link
Member Author

zcorpan commented Apr 27, 2022

Thanks @tidoust !

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

No branches or pull requests

3 participants