-
Notifications
You must be signed in to change notification settings - Fork 34
Bug 1719315 - Implement the URL metric type #529
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
Conversation
glean/src/core/metrics/types/url.ts
Outdated
| throw new UrlMetricError(ErrorType.InvalidValue, `${v} is not a valid URL.`); | ||
| } | ||
|
|
||
| if (url.protocol === "data:") { |
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.
This is not part of the proposal, but following the same rationale that led us to disallow data: URLs, we should also be disallowing blob: URLs.
a2c4214 to
fe03b7a
Compare
|
Argh, check size fails because of #532 We can retry once that is merged and this is rebased. Or not :) |
badboy
left a comment
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.
Looks good.
Co-authored-by: Jan-Erik Rediger <badboy@archlinux.us>
Build size report
|
Pull Request checklist
glean/folder, run:npm run testRuns all testsnpm run lint && npm run lint:circular-depsRuns all lintersCHANGELOG.mdor an explanation of why it does not need one...Also missing:
Both these changes block this PR, before them the integration tests will not pass.