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

Add spec URLs for the web/svg subtree #13265

Merged
merged 3 commits into from
May 13, 2022

Conversation

sideshowbarker
Copy link
Collaborator

@sideshowbarker sideshowbarker commented Feb 24, 2022

Part of #13126. Many of these documents already had a browser-compat key — and thus didn’t need a spec-urls key. So in those cases, this change just replaces the manual specifications table with a {{Specifications}} macro.

@sideshowbarker sideshowbarker requested a review from a team February 24, 2022 22:46
@github-actions github-actions bot added the Content:SVG SVG docs label Feb 24, 2022
@github-actions

This comment was marked as outdated.

Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's so great to see all these HTML tables gone.

My only concern here is that some of these pages have BCD, but the BCD is missing spec URLs, so the pages are losing spec tables in the transition.

We could just file a bug against BCD listing all pages that need spec URLs, and the URLs they need, scraped out of these pages? I'd be happy to help add them.

@sideshowbarker
Copy link
Collaborator Author

What I think ought to happen here is that we add the spec URL to BCD

I went ahead and opened PR mdn/browser-compat-data#15114 for that, with patch.

This one (at least this one, I haven't checked all of them) has BCD but it's missing spec-urls

Thanks for having caught that. I had a bug/oversight in the code I wrote to add the spec_url values: My code assumed that if a document had a browser-compat value, the data for whatever BCD feature it listed must have a spec_url. In other words, the code wasn’t checking if the data for th specified feature actually did had a spec_url. So I updated it to do that check, and then re-ran it.

There turned out to be 60 cases where a specified BCD feature didn’t have a spec_url value. In every case, the feature also didn’t have an mdn_url value. So I opened mdn/browser-compat-data#15114 to get those added.

37 of those features are specified in the SVG2 spec or another current spec.
23 of those features are specified in the SVG11 spec only, and are therefore obsolete. And I think in many or most of those cases, they are features that were never implemented in any browser.

I’m personally not super-enthusiastic about us going out of our way to maintain data on features that never actually got implemented in any browsers. I’d be happier to see them removed from BCD completely, and for the MDN articles to disappear too.

But anyway, the mdn/browser-compat-data#15114 patch does add the spec_url and mdn_url values for those obsolete features too — but I put those changes into a separate commit in the PR, on the off chance that we might decide we’re better off dropping those features from BCD.

@sideshowbarker
Copy link
Collaborator Author

My only concern here is that some of these pages have BCD, but the BCD is missing spec URLs, so the pages are losing spec tables in the transition.

Along with the cases where we had BCD data for a feature but that data was missing a spec_url (the cases fixed in mdn/browser-compat-data#15114), there were 10 cases where we have a browser_compat key in the document, but the feature given in the value of that key doesn’t actually exist in BCD.

Maybe those are features that should exist in BCD or maybe they shouldn’t — or maybe they were intentionally removed.

Regardless, I don’t think we should block this PR on getting those features added to BCD, because it’s a much bigger deal to add all the data for a feature to BCD — we’d need to investigate the browser support for it, and add all the browser-version data (not just the spec_url data).

Somebody should ideally do that eventually (I’m not volunteering) but I suggest that for now now we just go ahead and add the spec-urls to the MDN documents — so that in the meantime before the features get added to BCD, we at least have the Specifications section for them in MDN.

So I pushed 39c9688 with a change that adds the spec-urls for them here.

@teoli2003
Copy link
Contributor

The problem of web/svg is that we have pages about attributes at a global level and the attributes are defined on several SVG elements. I'm not sure we want a page per attribute, but if so, we should have the attribute pages below their element. Right now there is no correspondence between what is in bcd (and that is incomplete) and pages on content. But fixing this means completing/updating the web/svg docs. That's a huge amount of work and probably not the top priority as most people are using tools to generate svg files.

@sideshowbarker
Copy link
Collaborator Author

But fixing this means completing/updating the web/svg docs. That's a huge amount of work and probably not the top priority as most people are using tools to generate svg files.

Yeah, there are some bigger problems with the SVG docs, but I think it’s a win when we can at least just make them a little better (in this case by getting rid of the manual specification tables — especially the HTML tables that the svg subtree seemed to have a lot of).

@wbamberg
Copy link
Collaborator

What I think ought to happen here is that we add the spec URL to BCD

I went ahead and opened PR mdn/browser-compat-data#15114 for that, with patch.

This one (at least this one, I haven't checked all of them) has BCD but it's missing spec-urls

Thanks for having caught that. I had a bug/oversight in the code I wrote to add the spec_url values: My code assumed that if a document had a browser-compat value, the data for whatever BCD feature it listed must have a spec_url. In other words, the code wasn’t checking if the data for th specified feature actually did had a spec_url. So I updated it to do that check, and then re-ran it.

There turned out to be 60 cases where a specified BCD feature didn’t have a spec_url value. In every case, the feature also didn’t have an mdn_url value. So I opened mdn/browser-compat-data#15114 to get those added.

Thank you!

I’m personally not super-enthusiastic about us going out of our way to maintain data on features that never actually got implemented in any browsers. I’d be happier to see them removed from BCD completely, and for the MDN articles to disappear too.

Yes, I agree, but that seems like scope creep for this PR.

Along with the cases where we had BCD data for a feature but that data was missing a spec_url (the cases fixed in mdn/browser-compat-data#15114), there were 10 cases where we have a browser_compat key in the document, but the feature given in the value of that key doesn’t actually exist in BCD.

Maybe those are features that should exist in BCD or maybe they shouldn’t — or maybe they were intentionally removed.

Regardless, I don’t think we should block this PR on getting those features added to BCD, because it’s a much bigger deal to add all the data for a feature to BCD — we’d need to investigate the browser support for it, and add all the browser-version data (not just the spec_url data).

Somebody should ideally do that eventually (I’m not volunteering) but I suggest that for now now we just go ahead and add the spec-urls to the MDN documents — so that in the meantime before the features get added to BCD, we at least have the Specifications section for them in MDN.

So I pushed 39c9688 with a change that adds the spec-urls for them here.

Yes, I agree that this is the right thing to do.

@sideshowbarker sideshowbarker force-pushed the sideshowbarker/spec-urls-web/svg branch from 39c9688 to 18896dd Compare February 27, 2022 09:35
@sideshowbarker sideshowbarker force-pushed the sideshowbarker/spec-urls-web/svg branch from 18896dd to 7154f0a Compare May 13, 2022 14:21
Copy link
Collaborator

@queengooborg queengooborg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, this is LGTM!

@queengooborg queengooborg merged commit 925f7e1 into main May 13, 2022
@queengooborg queengooborg deleted the sideshowbarker/spec-urls-web/svg branch May 13, 2022 21:47
hamishwillee pushed a commit to hamishwillee/content that referenced this pull request May 23, 2022
* Add spec URLs for the web/svg subtree

Part of mdn#13126

* Drop browser-compat for SVG cases not actually in BCD; add spec-urls instead

* Fix browser-compat for “by”, “spreadMethod” & “vector-effect”

Related: mdn/browser-compat-data#15141
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:SVG SVG docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants