mdn / browser-compat-data

This repository contains compatibility data for Web technologies as displayed on MDN
https://developer.mozilla.org
Creative Commons Zero v1.0 Universal
4.99k stars 2.01k forks source link

Linter replaces MDN url anchor when page is now redirecting to anchor #25147

Open caugner opened 3 hours ago

caugner commented 3 hours ago

What type of issue is this?

Linter issue

What is the issue?

If a feature has an mdn_url that points to an anchor of an MDN page, and that page is removed and redirected to another page with an anchor, then this redirect target replaces the mdn_url and the current anchor gets lost, even if the redirect target has an anchor corresponding to the old mdn_url.

See: https://github.com/mdn/browser-compat-data/pull/25138/commits/4c58e86a0317f28a7ada2d5f9d6daa5f55f711f0

diff --git a/http/headers/Content-Security-Policy.json b/http/headers/Content-Security-Policy.json
index 6c1ee46a99cd45..64dc4a85df7bdc 100644
--- a/http/headers/Content-Security-Policy.json
+++ b/http/headers/Content-Security-Policy.json
@@ -746,7 +746,7 @@
         "report-sample": {
           "__compat": {
             "description": "`report-sample` source value",
-            "mdn_url": "https://developer.mozilla.org/docs/Web/HTTP/Headers/Content-Security-Policy/Sources#report-sample",
+            "mdn_url": "https://developer.mozilla.org/docs/Web/HTTP/Headers/Content-Security-Policy#fetch_directive_syntax",
             "support": {
               "chrome": {
                 "version_added": "59"
@@ -1152,7 +1152,7 @@
         "strict-dynamic": {
           "__compat": {
             "description": "`strict-dynamic` source value",
-            "mdn_url": "https://developer.mozilla.org/docs/Web/HTTP/Headers/Content-Security-Policy/Sources#strict-dynamic",
+            "mdn_url": "https://developer.mozilla.org/docs/Web/HTTP/Headers/Content-Security-Policy#fetch_directive_syntax",
             "support": {
               "chrome": {
                 "version_added": "52"
@@ -1338,7 +1338,7 @@
         "unsafe-hashes": {
           "__compat": {
             "description": "`unsafe-hashes` source value",
-            "mdn_url": "https://developer.mozilla.org/docs/Web/HTTP/Headers/Content-Security-Policy/Sources#unsafe-hashes",
+            "mdn_url": "https://developer.mozilla.org/docs/Web/HTTP/Headers/Content-Security-Policy#fetch_directive_syntax",
             "support": {
               "chrome": {
                 "version_added": "69"

What behavior were you expecting?

We should have not overwritten these specific MDN urls.

What version(s) of BCD is the issue present in?

Do you have anything more you want to share?

No response

caugner commented 3 hours ago

This specific occurrence seems to have been a side-effect of https://github.com/mdn/content/pull/36792, where /docs/Web/HTTP/Headers/Content-Security-Policy/Sources was removed and redirected to docs/Web/HTTP/Headers/Content-Security-Policy#fetch_directive_syntax.

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy#report-sample exists, so what we should have done in this case almost is to fetch the page, look if an id corresponding to the current hash exists, and use that.

/cc @fiji-flo It would be useful, if the index.json contained a list or map of all anchors of the page. Is this something you have considered, or would consider for a future version of rari? (Side-note: This would also be useful for "redirecting" en-US anchors on other locales to the localized anchor, which I have mentioned recently.)

fiji-flo commented 3 hours ago

Anchors are a lovely topic. One of my biggest goals is to move to GFM like anchors which would help a lot.

I don't want to add more redundant data to the index.json. However, validating id/anchors and using them for redirects and and other task is on the agenda.

For this case it would not be straight forward, just because and id (and thereby a heading) matches doesn't imply that where we should point. There's lot of pages that have the same id many times (with a unique added suffix) which makes this even worth.

Thanks for raising this.

caugner commented 3 hours ago

Thanks Florian.

@queengooborg Maybe what we should do is to change the behavior as follows: _Error if the current mdn_url contains an anchor, and don't auto-fix it._ Wdyt?