Skip to content

Conversation

@brizental
Copy link
Contributor

@brizental brizental commented Jul 13, 2021

Pull Request checklist

  • Quality: Make sure this PR builds and runs cleanly.
    • Inside the glean/ folder, run:
      • npm run test Runs all tests
      • npm run lint && npm run lint:circular-deps Runs all linters
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Changelog: This PR includes a changelog entry to CHANGELOG.md or an explanation of why it does not need one
  • Documentation: This PR includes documentation changes, an explanation of why it does not need that or a follow-up bug has been filed to do that work

...Also missing:

Both these changes block this PR, before them the integration tests will not pass.

throw new UrlMetricError(ErrorType.InvalidValue, `${v} is not a valid URL.`);
}

if (url.protocol === "data:") {
Copy link
Contributor Author

@brizental brizental Jul 13, 2021

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.

@brizental brizental force-pushed the 1719315-url branch 2 times, most recently from a2c4214 to fe03b7a Compare July 13, 2021 17:19
@brizental
Copy link
Contributor Author

Argh, check size fails because of #532

We can retry once that is merged and this is rebased. Or not :)

@brizental brizental requested a review from badboy July 15, 2021 16:06
Copy link
Member

@badboy badboy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@moz-glean
Copy link
Collaborator

Build size report

Merging #529 into main will:

  • Leave the size of full Web Extension bundle unchanged.
  • Increase the size of full Qt/QML bundle build by 3%.

Current size New size Size increase
Web Extension
core only 45 KB 45 KB 📈 546 bytes
full bundle 69 KB 70 KB 📈 546 bytes
Qt/QML
core only 110 KB 113 KB 📈 3.5 KB
full bundle 110 KB 113 KB 📈 3.5 KB

@brizental brizental merged commit f811d53 into mozilla:main Jul 16, 2021
@brizental brizental deleted the 1719315-url branch July 16, 2021 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants