-
Notifications
You must be signed in to change notification settings - Fork 387
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
Remove the marker deriving step, and remove processing for Bailout and Invalidation #2851
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2851 +/- ##
==========================================
+ Coverage 87.39% 87.50% +0.11%
==========================================
Files 235 234 -1
Lines 18788 18700 -88
Branches 4770 4757 -13
==========================================
- Hits 16420 16364 -56
+ Misses 2189 2161 -28
+ Partials 179 175 -4
Continue to review full report at Codecov.
|
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.
Thanks for the clean-up, this seems to work just fine!
Let's see if our users want the structured version back...
Crazy idea: maybe using the new "naming group" functionality in regexps we could be able to support such a processing purely in the schema
{
structuringRegexp: "Invalidate (?<url>[\w/_-]+):(?<lineNumber>\d+):(?<columnNumber>\d+)",
structuringRegexpApplyTo: "name",
tooltipLabel: "{regexp.groups.url} at line {regexp.groups.lineNumber} and column {regexp.groups.columnNumber}",
}
(crazy idea I said)
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.
Crazy idea: maybe using the new "naming group" functionality in regexps we could be able to support such a processing purely in the schema
Part of my thought process on the marker schema is that it can be an attack vector. I'm hesitant to open things up where we're executing regexes from random user data. I'd prefer to make the data structured if we can.
c0b7d60
to
22f3cbb
Compare
22f3cbb
to
e30c7b8
Compare
before - after - Firefox 72 - Working derived bailout markers, no payload marker
before - after - Firefox 82 - Broken derived bailout markers, no payload marker
before - after - Firefox 83 - Text payload bailout markers
Our bailout markers are very divergent in the forms. Combined with the marker schema changes, things aren't working as expected. This PR strips out all of the special handling for markers, and completely relies on Gecko for the display.