Update the BrowserCompat component to filter against stable browsers

[tannerdolby]

Fixes #7447

Changes proposed in this pull request:

• Introduce a ReleaseStatement to lookup status info for a given browser release version within compatAria, compatVersion, compatProperty functions.
• Perform a status check to determine if the browser release is "current" or not when deciding whether to display the version number or "x" in the compat* functions.

Notes:
The BrowserCompat.js component before my proposed changes was mainly relying on SimpleSupportStatement which provided version_added, version_removed etc. But this statement type did not include the fields we needed to check the browser version status, as this information was stored in ReleaseStatement.

Since we needed to get ahold of a ReleaseStatement along with the SimpleSupportStatement, I added a bit of logic to do a lookup for a given browser release version provided a support object to get the version_added, and then check if that release version has a "current" status (not beta). This seems to do the trick for filtering only stable browser release versions, but there is definitely room for improvement for more sturdy filtering based on the possible status values in ReleaseStatement object.

For example, the scrollbar-gutter property displays a browser release version "94" which corresponds to a "retired" status meaning the release is no longer supported, but Chrome 94 does support scrollbar-gutter according to caniuse/scrollbar-gutter, so this case was something I wanted to mention.

[jeffposnick]

Thank you for the contribution!

Checking for release.status === 'current' does not appear to give the exact behavior we want here, since it excludes features that were added in an earlier browser release.

I.e. Firefox 94 is not "current," but features that were added in Firefox 94 are widely available.

The production rendering for something added in Firefox 94 looks good:

With this PR, the feature added in Firefox 94 incorrectly ends up with an X:

I think you need to check for release.status === 'current' || release.status === 'retired' to get the expected behavior.

[tannerdolby]

Hello @jeffposnick! No problem.

Checking for release.status === 'current' does not appear to give the exact behavior we want here, since it excludes features that were added in an earlier browser release.

Yeah, this is what I was unsure about. At first, I checked "current" || "retired" as I noticed there were more small "x"s than the original page when only filtering against "current" releases, but ultimately even if a browser is retired it still can have widely available/used features so filtering against both is a better option.

I will add the release.status === 'current' || release.status === 'retired' check like you mentioned.

[jeffposnick]

This logic seems good to me.

I'll defer to @rachelandrew for the final review, since she opened the issue to begin with.

[tannerdolby]

@jeffposnick Ok, great! Thanks.

[tannerdolby]

@rachelandrew Thanks for reviewing/merging this. I see the PR has been reverted here, was there an issue breaking the build? I'm happy to fix any issues to get this feature request landed.

The build was fully passing when originally created around 3 months ago, maybe some logic is conflicting or changes have been made which I didn't account for since this PR was started. I looked at the Percy step of build process and see the following:

[11ty] 1. Having trouble rendering njk template ./src/site/content/en/blog/building-a-button-component/index.md (via TemplateContentRenderError)
[11ty] 2. (./src/site/content/en/blog/building-a-button-component/index.md)
[11ty]   EleventyShortcodeError: Error with Nunjucks shortcode `BrowserCompat` (via Template render error)
[11ty] 3. Cannot read properties of undefined (reading 'status') (via Template render error)

[rachelandrew]

Yes I was getting the status error message locally, and it broke the production build. As I couldn't see why I reverted it pending someone from the infra team investigating,

[tannerdolby]

Ok, thank you for the heads up. Let me know if I can do anything to help.

[devnook]

Hi @tannerdolby thanks for your contribution!

This PR problem was that it didn't account for non-standard data in MDN BCD, e.g. here: https://github.com/mdn/browser-compat-data/blob/main/html/elements/button.json ("version_added: true"). To fix this, we'd need to add logic for a case where release data (and therefore its status) cannot be found.
We'd need to add this additional logic to the test file too, to avoid such issues in future (https://github.com/GoogleChrome/web.dev/blob/main/test/unit/src/site/_includes/components/BrowserCompat.js)
Finally, we might consider adding this logic in _data instead (not sure if it makes sense though, just a suggestion).

I cannot commit these changes to your fork, so please let me know if you'd like to fix these issues and submit a new PR, or if you'd prefer that I do it instead (in a new PR too).

@rachelandrew @jeffposnick This PR broke our tests, but it was not caught in time because the $_presubmit label was not added before merging. Please add $_presubmit label to trigger tests before merging a js logic PR.

[tannerdolby]

Hi @devnook no problem, thanks for triaging this PR!

Ah, that makes sense. Good catch. Now that I look back at the code and mdn-compat-data, its definitely clear the logic I proposed didn't handle non-standard data.

Thank you for the suggestions! I will go ahead and create a new PR then we can go from there. I can try experimenting locally to see if adding this logic as a global data file in _data would make sense. It seems fine in its current state within _includes/components/ but I need to get re-familiarized with the web.dev project structure again before providing more detailed input.