-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Validate spec_urls based on webref ids #23958
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
base: main
Are you sure you want to change the base?
Conversation
I'd say that the good news is that, in most cases, it seems that "something else is going on" ;) Main categories of errors I see:
And then there are actual broken links in BCD, such as https://tc39.es/proposal-temporal/#sec-get-temporal.zoneddatetime.prototype.timezone. There are also "outdated" URLs, such as https://tc39.es/ecma262/multipage/additional-ecmascript-features-for-web-browsers.html#sec-object.prototype.__defineGetter__, which redirects to https://tc39.es/ecma262/multipage/fundamental-objects.html#sec-object.prototype.__defineGetter__ that appears in Webref. There may be a few other error cases to dig into. |
Fantastique François!! 🎉 What I see now:
Something I would like for you to take a look:
|
As far as I can tell, all of them are examples of what I called outdated links: they work, but that's because the HTML spec has logic in place to redirect past fragments to their new page. Each time, the content referenced by the link moved to another page of the HTML spec and would better be targeted using the new fragment to avoid a redirect. For example, clicking on https://html.spec.whatwg.org/multipage/browsing-the-web.html#dom-beforeunloadevent-returnvalue makes you load the |
For context, see discussion starting at: mdn/browser-compat-data#23958 (comment) Some specs such as DOM, Encoding, HTML contain sections targeted at web developers. These sections re-define terms normatively defined elsewhere in a more developer-friendly way. Terms re-defined in these sections are good targets for documentation but did not appear in definitions extracts. This update makes Reffy parse "for web developers" sections and extract the definitions they contain. This is a prerequisite to publishing a package with definitions that could be used to validate URLs in BCD and web-features, as envisioned in: w3c/webref#1198 (comment) Worth noting: - Ideally, spec authoring tools would provide better support for this pattern, giving definitions more stable IDs than `ref-for-[foo][number]` and creating proper dfns themselves. If they do that, the custom processing introduced here would become moot. Going through tools and specs will take time though. - To keep the cross-references database useful, newly extracted definitions need to be in a separate dfn namespace, i.e., have their own dfn type. Problem is that they also have a "natural" dfn type such as `interface`, `method` or `attribute`. The solution implemented here is to prefix their type with `dev-`. That duplicates dfn types. A cleaner solution would record the "dev" namespace in another property. But that would surprise spec authoring tools. An alternative approach would be to give all of these dfns a `dev` dfn type, but we'd then lose information that could turn out to be useful. - The key marker for sections targeted at web developers is the use of a `domintro` class. Now, a few specs do use `domintro` in normative definition lists (shape-detection-api, image-capture, mediastream-recording). That's probably unintentional. I'll look into fixing the specs. The code skips `domintro` sections that look suspicious. - This would add **2815 definitions** to the dfns extracts (which currently contain ~50000 definitions)
For context, see discussion starting at: mdn/browser-compat-data#23958 (comment) Some specs such as DOM, Encoding, HTML contain sections targeted at web developers. These sections re-define terms normatively defined elsewhere in a more developer-friendly way. Terms re-defined in these sections are good targets for documentation but did not appear in definitions extracts. This update makes Reffy parse "for web developers" sections and extract the definitions they contain. This is a prerequisite to publishing a package with definitions that could be used to validate URLs in BCD and web-features, as envisioned in: w3c/webref#1198 (comment) Worth noting: - Ideally, spec authoring tools would provide better support for this pattern, giving definitions more stable IDs than `ref-for-[foo][number]` and creating proper dfns themselves. If they do that, the custom processing introduced here would become moot. Going through tools and specs will take time though. The custom processing done here allows to add the definitions right away. It does not solve the "unstable" IDs issue, but at least provides a theoretical way to identify situations where the ID of a dev dfn changes. - To keep the cross-references database useful, newly extracted definitions need to be in a separate dfn namespace, i.e., have their own dfn type. Problem is that they also have a "natural" dfn type such as `interface`, `method` or `attribute`. The solution implemented here is to prefix their type with `dev-`. That duplicates dfn types. A cleaner solution would record the "dev" namespace in another property. But that would surprise spec authoring tools. An alternative approach would be to give all of these dfns a `dev` dfn type, but we'd then lose information that could turn out to be useful. - The key marker for sections targeted at web developers is the use of a `domintro` class. Now, a few specs do use `domintro` in normative definition lists (shape-detection-api, image-capture, mediastream-recording). That's probably unintentional. I'll look into fixing the specs. The code skips `domintro` sections that look suspicious. - This would add **2815 definitions** to the dfns extracts (which currently contain ~50000 definitions)
For context, see discussion starting at: mdn/browser-compat-data#23958 (comment) Some specs such as DOM, Encoding, HTML contain sections targeted at web developers. These sections re-define terms normatively defined elsewhere in a more developer-friendly way. Terms re-defined in these sections are good targets for documentation but did not appear in definitions extracts. This update makes Reffy parse "for web developers" sections and extract the links that complete definitions they contain. This is a prerequisite to publishing a package with definitions that could be used to validate URLs in BCD and web-features, as envisioned in: w3c/webref#1198 (comment) The links are recorded in a `links` property attached to the base definition that the link completes. The `links` property is an array of objects, each object featuring `id`, `href`, `type`, `name` and `heading` properties. The `type` property is always set to `"dev"`. The `name` property contains the text content of the enclosing `<dt>`. The `heading` property contains the heading of the section where the anchor is defined (it may be different from the heading of the section where the underlying definition appears). There may be more than one dev link per definition. That's normal. It typically happens when the underlying definition is for a mixin included in multiple interfaces, as for `TextDecoderCommon` attributes in the Encoding spec. Some links for developers target definitions in external specs. They are ignored for now. Worth noting: - Ideally, spec authoring tools would provide better support for this pattern, giving these links more stable IDs than `ref-for-[foo][number]` and possibly creating proper dfns themselves. If they do that, processing may need to be adjusted. Updating tools and specs will take time though. - The key marker for sections targeted at web developers is the use of a `domintro` class. Now, a few specs do use `domintro` in normative definition lists (shape-detection-api, image-capture, mediastream-recording). That's probably unintentional. I'll look into fixing the specs. The code skips `domintro` sections that look suspicious. - This would add **2815 links** to the dfns extracts (for ~50000 definitions)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dfns in Webref now contain production rules from the ECMAScript spec, and links that are "for web developers". The latter requires minor adjustments to the code, see inline.
Running the linter locally, this gets us down to 405 links that fail validation and that target 319 different fragments.
This includes 29 new links to ECMAScript proposals that got integrated in the main ECMAScript spec. We've flagged the specs as "discontinued" in browser-specs accordingly and links should be updated in BCD. I guess that shows the merit of running such a validation ;)
I'll look into the 111 #ref-for
links which, as far as I can tell, all target IDL blocks. I may be able to extend the newly introduced links
mechanism with these.
I noticed a few RFCs missing from browser-specs and that could probably be added. (Edit: They're not missing in practice but extraction of headings seems to fail for some reason. Anyway, I'll look into it)
I also note that api.Element.attachShadow
links to https://dom.spec.whatwg.org/#ref-for-dom-element-attachshadow① and... that seems to be a good example of a link that changed without anyone noticing? I suspect the link should rather target https://dom.spec.whatwg.org/#ref-for-dom-element-attachshadow②
Co-authored-by: François Daoust <fd@tidoust.net>
For links to IDL terms, specs may follow two distinct patterns:
I don't think there's broad agreement on which approach is the right one. I note that BCD does not seem to be consistent either, and contains links that target either the IDL block or the prose. Typically, among links that fail validation, there are both:
Question is: from a BCD perspective, what would you like links to target for IDL terms? The IDL block or the prose? Practically speaking, I can easily extract the reference in the IDL block when the second pattern is used. I do not see an easy way to extract the "main" reference in prose when the first pattern is used. Perhaps a rule such as "first normative occurrence right after the IDL block" could work though. As usual, targeting references that are not qualified in any way is not fantastic: |
Co-authored-by: François Daoust <fd@tidoust.net>
We usually use anchors in the form of So, #23958 (comment) and ~25% of the failures should get fixed by #27293. |
Ah, that certainly works for me ;) I assumed that you would want to treat these anchors like the ones that point at "for web developers" sections, meaning that you were more looking at pointing at a specific place than at always following what the underlying spec uses as main definition anchor. |
I think it is fine but I would certainly welcome if spec authors could agree on consistency for |
Down to 207 issues! Seems like lots of problems with SVG and WebGL at this point. |
WebGL1 problems should disappear once a new version of Reffy gets released (that's pending release of a new version of webidl2.js with support for the new For SVG links, the specs need an overhaul. I'm reluctant to spend time in Reffy to try to make sense of IDs in these specs in the meantime (for example, for IDL term definitions, we would need to parse the IDL to make sense of the definitions, but the IDL in SVG Paths is currently invalid, so we would need to patch it first). May I suggest skipping checks for links that target @Elchi3, the remaining ~100 links seem to be mainly things that need to be reviewed one by one. I see links where specs could perhaps be updated. Others where BCD probably should. There will no doubt be a few ones that need to be handled as exceptions to the rule. I'd be happy to hop on the phone and go through them with you. Feel free to ping me if you would fancy that! |
Thanks @tidoust! Now we're down to 53! I will look through them and will probably have questions for you :) |
Draft testing PR for @tidoust :)
Based on w3c/webref#1198 (comment), I wrote a quick test to see if webref ids could be used to (deeply) validate BCD's spec_urls. (that is, we want to check if the fragment ids are valid as well, not just the spec hosts).
It spits out a lot of errors and I would be interested to hear if BCD should be using different fragment ids, or if webref is missing these fragment ids, or if something else is going on. Please see the CI failure for the results.
(This is a draft PR that removes our dependency on web-specs and instead fetches raw webref JSON files, we might not want to fetch the data this way, so consider this PR just a test for now)