Preserve ranges in mirroring #23856
Preserve ranges in mirroring #23856
Conversation
github-actions
bot
added
infra
| } | ||
| if (sourceVersion.replace('≤', '') in browserData.releases) { | ||
| return sourceVersion; | ||
| } | ||
| throw new Error(`Cannot find iOS version matching Safari ${sourceVersion}`); | ||
| } | ||
|
|
||
| const releaseKeys = Object.keys(browserData.releases); | ||
| releaseKeys.sort(compareVersions); | ||
| const versions = Object.keys(browserData.releases); |
There was a problem hiding this comment.
I changed this so that the local variable would be v both for Safari above and everything else below. Makes it easier to see that the condition around first release is the same.
| sourceRelease.engine_version && | ||
| compare(release.engine_version, sourceRelease.engine_version, '>=') | ||
| ) { | ||
| return r; |
There was a problem hiding this comment.
This is the key line. Previously ranges were never propagated here, and now they are except for the first release.
| }; | ||
|
|
||
| const mirrored = mirrorSupport('edge', support); | ||
| // Uncertain early Chrome support doesn't imply possible EdgeHTML |
There was a problem hiding this comment.
I was a bit concerned about introducing ranges where there might not be any uncertainty, so I looked at what would be different if Edge 79 doesn't get ranges. Happily, only these 22 paths are affected:
css.properties.filter.svg_elements
html.elements.iframe.sandbox.allow-forms
html.elements.iframe.sandbox.allow-pointer-lock
html.elements.iframe.sandbox.allow-same-origin
html.elements.iframe.sandbox.allow-scripts
html.elements.iframe.sandbox.allow-top-navigation
http.data-url.html_files
http.headers.Content-Security-Policy.form-action.blocks_redirects
http.mixed-content
http.mixed-content.allow_file_urls
http.mixed-content.allow_localhost_url
http.mixed-content.allow_loopback_url
svg.global_attributes.fill-rule
svg.global_attributes.filter
webextensions.api.permissions.Permissions
webextensions.api.permissions
webextensions.api.permissions.contains
webextensions.api.permissions.getAll
webextensions.api.permissions.onAdded
webextensions.api.permissions.onRemoved
webextensions.api.permissions.remove
webextensions.api.permissions.request
Fixing the data in these cases is probably better than adding a rule about transitions from non-WebKit to Blink engines.
There was a problem hiding this comment.
Some ranges removed in #23858.
There was a problem hiding this comment.
#23865 fixes filter for SVG elements. That leaves only the HTTP, SVG and WebExtensions data.
|
Tests are failing because this changes when mirroring can be used. Let's make sure we like the new behavior before I update the data to pass the tests. |
There was a problem hiding this comment.
LGTM overall. Thanks for fixing this! Nit to test Firefox Android with more interesting ranges.
The proposed fixes from CI make sense to me. Many can be fixed with npm run fix and for the consistency errors, we should remove the ranges from the upstream browser:
- api.RTCStatsReport.type_inbound-rtp.*: chrome should be 79
- css.properties.line-break.*: firefox should be 69
| it('firefox_android', () => { | ||
| const support: InternalSupportBlock = { | ||
| firefox: { | ||
| version_added: '≤100', |
There was a problem hiding this comment.
| version_added: '≤100', | |
| version_added: '≤79', |
| }; | ||
|
|
||
| const mirrored = mirrorSupport('firefox_android', support); | ||
| assert.deepEqual(mirrored, { version_added: '≤100' }); |
There was a problem hiding this comment.
| assert.deepEqual(mirrored, { version_added: '≤100' }); | |
| assert.deepEqual(mirrored, { version_added: '≤79' }); |
| const mirrored = mirrorSupport('firefox_android', support); | ||
| assert.deepEqual(mirrored, { version_added: '≤100' }); | ||
| }); | ||
|
|
There was a problem hiding this comment.
| it('firefox_android', () => { | |
| const support: InternalSupportBlock = { | |
| firefox: { | |
| version_added: '≤69', | |
| }, | |
| }; | |
| const mirrored = mirrorSupport('firefox_android', support); | |
| assert.deepEqual(mirrored, { version_added: '≤79' }); | |
| }); |
Fixes #23855.