-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Firefox 139 removes SVG discard element again #26883
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
Firefox 139 removes SVG discard element again #26883
Conversation
|
Tip: Review these changes grouped by change (recommended for most PRs), or grouped by feature (for large PRs). |
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.
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.
@Elchi3 @ddbeck For context, this was accidentally exposed for 4 or so FF releases as part of an experiment, when the team misunderstood that Chrome are not interested in this and proposed it out of the spec. If they hadn't done this, I would now be removing it. Very much doubt anyone would have used this, as its a bit buggy.
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.
I'd prefer to follow the guidelines and remove it when the guidelines say to. I appreciate that this is a bit of an oddity (a nuisance even), but as a consumer, the consistency is nice to have. Also, if this ever gets revived soon, the we'll avoid hassles like we've seen in #26659.
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.
@ddbeck FWIW this won't be revived - chrome requested it be removed from the spec, which was a fact that the FF team missed - which is why they added support.
It's ready to merge if this is the decision ^^^
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.
but as a consumer, the consistency is nice to have
@ddbeck Can you explain the impact for consumers if we remove this now versus in 2 years?
I don't think this is comparable to the Sanitizer API, because that API appears to have been intentionally exposed in Chrome, whereas this interface was accidentally exposed in Firefox, with the rest of the feature being behind a flag.
The way I see it is the guidelines should guide our actions, yet leave us some flexibility for cases that were likely not considered when writing the guidelines.
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.
In general, removals and renames are a pain—they are the highest friction part of regular BCD upgrades. There's no good way to handle them automatically because they leave no trace. And because they can happen at any time, BCD's version number is not a good signal about the difficulty of a BCD upgrade. I feel a pang of annoyance on every BCD upgrade, not knowing how much work it is going to be. (I'm increasingly of the mind that every key removal ought to be semver major or not done at all.)
But in this particular case, the removal is higher friction than usual (when it appears in the eventual release). While the PR title will say "Firefox 139 remove[d]" the feature, the diff (both here and of the summary data I generate from BCD) will look as if the feature never existed or was de-duplicated with respect to another key. If I hadn't already read this PR, I'd probably have to read through it to understand what's going on (since I do know the guidelines and have an intuition about the circumstances for a key's removal).
that API appears to have been intentionally exposed in Chrome, whereas this interface was accidentally exposed in Firefox
I think this is no justification at all. I trust BCD to tell me facts about the APIs that the covered browsers expose, not to tell me about the intentions of their developers (or to cover up their mistakes).
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.
I understand your pain points about removals and renames, but I still don't understand what difference it makes in this particular case if we remove this feature now versus in 2 years? I can imagine that it would make a difference if a) the feature gets re-implemented, or b) your pain/friction/annoyance gets resolved in the mean-time (e.g. #24234).
In this particular case, we would remove the feature along with svg.elements.discard.
@ddbeck Can you walk me through your BCD upgrade process with a focus on the friction in this particular case? I would like to understand the underlying problem.
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.
The compromise version of this would be to ship a BCD release with the version_removed data added. Then, at least one release later, remove the feature itself and note that it was an accelerated removal in the release notes. That would help with the upgrade friction, because it would look more "normal" for an unsupported feature to disappear (instead of a seemingly-supported feature to disappear).
But I'm going to be out of office for two weeks, so do whatever and don't block on me.
1d59243 to
fbb99ad
Compare
|
This pull request has merge conflicts that must be resolved before it can be merged. |
fbb99ad to
c11f3d8
Compare
|
Thanks for merging @caugner - so re the discussion in #26883 (comment) , do we remove the API next release (and if so, do you want me to create a PR for that?) |
@hamishwillee Yes, let's remove that feature in the next release. If you could open a PR, that would be awesome. 🙏 |
FF139 removes experimental support for the SVG
<discard>element added behind pref in FF136, and theSVGDiscardElementinterface added to release in FF136 in https://bugzilla.mozilla.org/show_bug.cgi?id=1958839I've removed the experimental feature entirely as it never released.
For
SVGDiscardElementI've added the remove version, which is technically correct. Note though, this was pretty unsusable, and I suspect unused. It should have been behind pref IMO. So you might want to delete that too.Lastly, these changed from deprecated to experimental in FF136, and this reverts them. This is currently in the standard, and is expected to be remove, but hasn't been yet.
Associated docs work can be tracked in mdn/content#39618