-
Notifications
You must be signed in to change notification settings - Fork 204
Add Baseline regression notes #3187
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?
Changes from all commits
34f003a
89c742b
fccb850
241e4e5
a96e279
f632460
e877da5
7fb07e2
29a5b9d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,9 +2,13 @@ name: font-variant-position | |||||
| description: The `font-variant-position` CSS property sets whether to use alternate glyphs for subscript and superscript text. | ||||||
| spec: https://drafts.csswg.org/css-fonts-4/#font-variant-position-prop | ||||||
| group: font-features | ||||||
| # TODO: https://github.com/web-platform-dx/web-features/issues/1971 | ||||||
| # Status changed: https://github.com/web-platform-dx/web-features/pull/1958 | ||||||
| # 2024-10-15 — low → false — Chrome, Edge, and Safari do not implement font synthesis for missing superscript or subscript glyphs. | ||||||
| # References: | ||||||
| # - https://issues.chromium.org/issues/352218916 | ||||||
| # - https://bugs.webkit.org/show_bug.cgi?id=151471 | ||||||
| notes: | ||||||
| - date: 2024-10-15 | ||||||
| category: baseline-regression | ||||||
| old_baseline_value: low | ||||||
| new_baseline_value: false | ||||||
| message: > | ||||||
| Chrome, Edge, and Safari do not implement font synthesis for missing superscript or subscript glyphs. | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This message is partially contradicted by the support data for Safari: web-features/features/font-variant-position.yml.dist Lines 9 to 10 in 7fb07e2
|
||||||
| citations: | ||||||
| - https://issues.chromium.org/issues/352218916 | ||||||
| - https://bugs.webkit.org/show_bug.cgi?id=151471 | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,12 +2,16 @@ name: Popover | |
| description: The `popover` HTML attribute creates an overlay to display content on top of other page content. Popovers can be shown declaratively using HTML, or using the `showPopover()` method. | ||
| spec: https://html.spec.whatwg.org/multipage/popover.html | ||
| group: html | ||
| # TODO: https://github.com/web-platform-dx/web-features/issues/1971 | ||
| # Status changed: https://github.com/web-platform-dx/web-features/pull/1797 | ||
| # 2024-09-18 — low → false — Safari on iOS has a bug that prevents dismissing popovers by touch. | ||
| # References: | ||
| # - https://github.com/mdn/browser-compat-data/issues/22927 | ||
| # - https://bugs.webkit.org/show_bug.cgi?id=267688 | ||
| # notes: | ||
| # - date: 2024-09-18 | ||
| # category: baseline-regression | ||
| # old_baseline_value: low | ||
| # new_baseline_value: false | ||
| # message: > | ||
| # Safari on iOS has a bug that prevents light dismiss (tapping outside the element to close it). | ||
| # citations: | ||
| # - https://bugs.webkit.org/show_bug.cgi?id=267688 | ||
| # - https://github.com/mdn/browser-compat-data/issues/22927 | ||
|
Comment on lines
+5
to
+14
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When I opened this PR, I had written this note. However, the feature has since progressed. It occurred to me that we should not show irrelevant notes, so I commented it out. This is a rather new idea, so I'd like to delete this entirely, if we decide this is the right way to go. Other options considered: some sort of "historic" flag on notes that are no longer relevant, or filtering historic notes from the published package. I figured both of those added complexity that wouldn't be very useful to anyone, least of all developers.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I almost feel like we should have a linting step in place which checks that |
||
| status: | ||
| compute_from: | ||
| - api.HTMLElement.popover | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,12 +2,6 @@ name: Streams | |
| description: The streams API creates, composes, and consumes continuously generated data. | ||
| spec: https://streams.spec.whatwg.org/ | ||
| group: streams | ||
| # TODO: https://github.com/web-platform-dx/web-features/issues/1971 | ||
| # Status changed: https://github.com/web-platform-dx/web-features/pull/2358, https://github.com/web-platform-dx/web-features/pull/2491 | ||
| # 2024-12-19 — low → false — Regressed status to match Caniuse, which considers support beginning at BYOB shipping. | ||
| # 2025-01-30 — false → high — Split BYOB into a separate "readable-byte-streams" feature. Linked that one to Caniuse. | ||
| # References: | ||
| # - https://caniuse.com/streams | ||
|
Comment on lines
-5
to
-10
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I fussed over this for a while and ended up deciding to omit the note entirely. The first note would say that the feature regressed, the second note would be some new category (advance? unregression?) showing that we more or less reverted the second note. It seemed to me that the correct course of action in that scenario would be to withdraw the note, so that's what I've done.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tend to agree. If our consumers really only care about baseline regressions, then we don't need a note here. Unless we have reasons to believe that the history of a feature would be useful. Which I don't think we do. |
||
| status: | ||
| compute_from: | ||
| - api.ReadableStream | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -4,13 +4,13 @@ import path from 'path'; | |||||
| import { Temporal } from '@js-temporal/polyfill'; | ||||||
| import { fdir } from 'fdir'; | ||||||
| import YAML from 'yaml'; | ||||||
| import { convertMarkdown } from "./text"; | ||||||
| import { GroupData, SnapshotData, WebFeaturesData } from './types'; | ||||||
|
|
||||||
| import { BASELINE_LOW_TO_HIGH_DURATION, coreBrowserSet, parseRangedDateString, getStatus } from 'compute-baseline'; | ||||||
| import { BASELINE_LOW_TO_HIGH_DURATION, coreBrowserSet, getStatus, parseRangedDateString } from 'compute-baseline'; | ||||||
| import { Compat } from 'compute-baseline/browser-compat-data'; | ||||||
| import { assertValidFeatureReference } from './assertions'; | ||||||
| import { convertMarkdown } from "./text"; | ||||||
| import { isMoved, isSplit } from './type-guards'; | ||||||
| import { FeatureData, GroupData, SnapshotData, WebFeaturesData } from './types'; | ||||||
|
|
||||||
| // The longest name allowed, to allow for compact display. | ||||||
| const nameMaxLength = 80; | ||||||
|
|
@@ -165,6 +165,13 @@ for (const [key, data] of yamlEntries('features')) { | |||||
| data.description = text; | ||||||
| data.description_html = html; | ||||||
| } | ||||||
| if (Array.isArray(data.notes)) { | ||||||
| for (const note of data.notes as FeatureData["notes"]) { | ||||||
| const { text, html } = convertMarkdown(data.description); | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This appears to copy the feature description into the note message. I think it should be something like this:
Suggested change
|
||||||
| note.message = text; | ||||||
| note.message_html = html; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // Compute Baseline high date from low date. | ||||||
| if (data.status?.baseline === 'high') { | ||||||
|
|
@@ -194,6 +201,15 @@ for (const [key, data] of yamlEntries('features')) { | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| // Ensure regression notes are still relevant | ||||||
| if (Array.isArray(data.notes)) { | ||||||
| for (const [index, note] of (data.notes as FeatureData["notes"]).entries()) { | ||||||
| if (note.new_baseline_value !== data.status.baseline) { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah cool! I see you've actually already done what I suggested in an earlier comment. |
||||||
| throw new Error(`regression note ${index} on ${key}.yml no longer applies (status is ${data.status.baseline}, note is ${note.new_baseline_value}). Delete this note.`); | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if (data.compat_features) { | ||||||
| // Sort compat_features so that grouping and ordering in dist files has | ||||||
| // no effect on what web-features users see. | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -136,6 +136,60 @@ | |
| "description": "Group identifiers", | ||
| "$ref": "#/definitions/Strings" | ||
| }, | ||
| "notes": { | ||
| "description": "Notes about this feature", | ||
| "items": { | ||
| "additionalProperties": false, | ||
| "description": "A note describing a Baseline status regression. For example, a feature that has moved from Baseline low to not Baseline.", | ||
| "properties": { | ||
| "category": { | ||
| "const": "baseline-regression", | ||
| "description": "The topic of this note. This field is also a discriminator for any future note types", | ||
| "type": "string" | ||
| }, | ||
| "citations": { | ||
| "description": "One or more URLs, such as bugs, used to justify the regression", | ||
| "items": { | ||
| "type": "string" | ||
| }, | ||
| "type": "array" | ||
| }, | ||
| "date": { | ||
| "description": "The date that the regression was added to web-features data", | ||
| "type": "string" | ||
| }, | ||
| "message": { | ||
| "description": "A short description of the cause of the regression as a plain text", | ||
| "type": "string" | ||
| }, | ||
| "message_html": { | ||
| "description": "A short description of the cause of the regression as HTML", | ||
| "type": "string" | ||
| }, | ||
| "new_baseline_value": { | ||
| "description": "The `baseline` status value after the regression", | ||
| "enum": ["low", false], | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How much value do we get in practice from letting the types exclude one of the possible Baseline statuses for old and new? Can we just use the generic definition and thus sidestep the stuff that quicktype is generating funny types for? |
||
| "type": ["string", "boolean"] | ||
| }, | ||
| "old_baseline_value": { | ||
| "description": "The `baseline` status value before the regression", | ||
| "enum": ["high", "low"], | ||
| "type": "string" | ||
| } | ||
| }, | ||
| "required": [ | ||
| "category", | ||
| "date", | ||
| "message", | ||
| "message_html", | ||
| "citations", | ||
| "old_baseline_value", | ||
| "new_baseline_value" | ||
| ], | ||
| "type": "object" | ||
| }, | ||
| "type": "array" | ||
| }, | ||
| "snapshot": { | ||
| "description": "Snapshot identifiers", | ||
| "$ref": "#/definitions/Strings" | ||
|
|
||
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.
Should the
baseline_low_datebe2025-08-11accordingly?web-features/features/createimagebitmap.yml.dist
Line 6 in 7fb07e2