-
Notifications
You must be signed in to change notification settings - Fork 5k
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
Version v8.0.7 RC #9160
Version v8.0.7 RC #9160
Conversation
Sync `master` with `develop`
…cOS. (#9075) * Fix popup/notification when browser is in fullscreen, primarily on OSX. The issue was reported internally via Slack. User was running Mac OSX Chrome in fullscreen mode where Chrome is created in a new Desktop workspace. The issue reproduced on OSX Chrome in fullscreen/maximized view overrides the explicitly set width and height for `windows.create()`. Possibly not overrides, but creates a window based off of the window that it was created from. Found a related [Chromium bug](https://bugs.chromium.org/p/chromium/issues/detail?id=263092&q=window%20create%20width%20os%3DMac&can=2). The fullscreen `popup.left` pixel will calculate the window position incorrectly since we set and assume the width of the created window. The incorrect `left` position the window and transition the focus Desktop/Workspace incorrectly and make is seem to lose focus of the new window/workspace. Incidentally this will make the popup full width/height, and create a new workspace for the view, which we have no control over until Chrome fixes it. This will check if the popup is 'fullscreen', which it gets passed from the origin window, if so then don't reposition the window. If Chrome fixes the issue we can revert this change. * Feedback commit Co-authored-by: Mark Stacey <markjstacey@gmail.com> Co-authored-by: Mark Stacey <markjstacey@gmail.com>
* convert requestUserApproval mock to wrapper
e2e tests will now reference the development MetaMetrics project instead of the production one. The metrics endpoint should be stubbed out during e2e tests anyway, but this seemed like a better default regardless.
The MetaMetrics project ID can now be set via environment variable. It has not been set yet in practice, so for now the old project IDs will still be used. This is in preparation for migrating to a new project.
In a non-production environment, Sentry was configured to send error reports to a "test" MetaMask project. It will still do this during e2e tests, but in development Sentry is now disabled completely. In practice this was never useful in development.
A missing substitution for a localized message will now only log an error upon the first occurrence. Further errors are generally not useful.
The Sentry DSN is now expected to be provided via environment variable for production builds. The build script will fail if it is missing, and an error will be thrown at runtime if it is missing. The `SENTRY_DSN` environment variable has been set in CI to the old value for `SENTRY_PROD_DSN`. We can migrate to a new DSN at some point in the future.
Our source maps were being corrupted during minification, because the `gulp-terser-js` plugin we were using didn't account for the existence of sourcemaps in the input. A configuration option to allow the input of sourcemaps was added in v5.2.0. The plugin has been updated, and we now use this option. Previously the generated sourcemaps had an invalid entry in the "sources" array, with the filename of the bundle itself. This was not a real source. After this change, this invalid source is no longer present.
This change updates the following two dependencies to address high severity advisories in the production dependencies: * Use elliptic@6.5.3 * Use dot-prop@5.2.0 The public advisories: - `elliptic`: [npm](https://www.npmjs.com/advisories/1547) - `dot-prop`: [npm](https://www.npmjs.com/advisories/1213), [GHSA-ff7x-qrg7-qggm](GHSA-ff7x-qrg7-qggm) I don't believe there to be any functional changes here: - I don't think we hit any (important?) codepaths of the whole `ipld-zcash/zcash-bitcore-lib/elliptic` subtree of 3Box - `dot-prop` doesn't have a changelog but; - Looking through [`v3.0.0...v4.0.0`](sindresorhus/dot-prop@v3.0.0...v4.0.0) it would seem that the breaking change was requiring Node.js 4 ([`88b6eb6`](sindresorhus/dot-prop@88b6eb6)) - The only breaking change listed for [v5.0.0](https://github.com/sindresorhus/dot-prop/releases/tag/v5.0.0) was requiring Node.js 8.
The `build-artifacts` directory is created on CI (as of #7151), and is used to store the sesify visualization and dependency logs. It's useful to have this ignored locally as well, for when those scripts are being tested.
The output remains identical between these two versions, and none of the changelog entries appear relevant to us (aside from maybe some of the bug fixes).
The changes between these versions don't affect us. The breaking change was related to passing in a function to `gulp-rename`, which we don't do.
The changes between v3.0.9 and v3.0.10 are minimial - just some minor improvements to error handling.
Background events are now sent in the `Background` category, rather than `backend`. Conventionally we use the term "background" over "backend", as it's not really a "backend" in the normal sense since it's a client background process. Also it's capitalized because all of the other event categories are capitalized as well. The metrics URL has been updated to use `background` instead of `backend` as well, for consistency. Luckily we don't have to worry about our metrics being disjointed due to this name change, because the background metrics never worked to begin with! So there will be none under the old name. The metrics will be made functional in a separate PR.
* Remove `url` parameter from `metricsEvent` The `url` parameter was used to override the `currentPath`, but it never worked correctly. It was supposed to be used for setting the `url` query parameter that was sent to Matomo, but `currentPath` was always used even if it `url` was set and `currentPath` was empty. Instead, `currentPath` is now always used. There was never a need to provide an "override" for `currentPath` when it can be set directly. The metrics provider does set `currentPath` automatically by default, but this can be overwritten already by passing a second parameter to `metricsEvent`. There were two places this `url` parameter was being used: background events, and path changes. Background events were submitted with no `currentPath`, so because of the bug with the `url` parameter, the metrics utility would crash upon each event. So those were never actually sent. This commit will fix that crash. The `currentPath` parameter was supplied as an empty string for the path change events, so those never crashed. They just had the `url` query string parameter set incorrectly (to an empty string). It should now be correctly populated, which should mean we'll be capturing all path changes now. Previously we were only capturing path changes to pages that happened to include an event, because of this blank `url` problem. * Use `url` query parameter as fallback for generating `pv_id` The `pv_id` parameter currently isn't generated correctly on Firefox, as the generation assumes that the current URL starts with `chrome-extension://`. The `url` query parameter is still unique for each path, so it's probably good enough for generating an id for each page. This is just a temporary fix; it will be removed in a future PR, where Firefox will be properly supported.
) The `currentPath` parameter passed to our metrics utility had been passed the full URL rather than just the path, contrary to what the name would imply. We only used the path portion, so passing the full URL did lead to complications. Now just the `pathname` is passed in, rather than the full URL. This simplifies the metrics logic, and it incidentally fixes two bugs. The main bug fixed is regarding Firefox metrics. Previously we had assumed the `currentPath` would start with `chrome-extension://`, which of course was not true on Firefox. This lead to us incorrectly parsing the `currentPath`, so path tracking was broken for Firefox events. This broken parsing is now bypassed entirely, so metrics should now work the same on Firefox as on Chrome. The second bug was that we were incorrectly setting the tracking URL for background events during tests. As a result, we were incorrectly detecting ourselves as an internal site that had referred the user to us. But this was not of major concern, since it only affected test metrics (which get sent to the development Matomo project). Lastly, this change let us discard the `pathname` parameter used in the `overrides` parameter of the `metricsEvent` function. Now that `currentPath` is equivalent to `pathname`, the `pathname` parameter is redundant.
* add web3 usage metrics * move web3 metrics method to new middleware * rename some methods, files, and exports
Builds ready [8063a83]
Page Load Metrics (593 ± 57 ms)
|
Changes that were not user-facing have been omitted, and the descriptions have been updated to be more readable for users.
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.
🚀
Builds ready [027723d]
Page Load Metrics (581 ± 42 ms)
|
I'm having trouble verifying that the metrics changes are working correctly; I have yet to demonstrate that the background web3 usage logs are making their way to Matomo |
@Gudahtt do they appear if you use the dev project? They did appear for me, although I don't think I checked since your metrics changes were merged. |
Builds ready [747f4bb]
Page Load Metrics (552 ± 56 ms)
|
It does look like the data is now being received by Matomo, though unfortunately the reports still have yet to update. The events are only viewable in the visitor log. |
I ran into that issue as well, but the update on Event updates are slow, and/or daily, check 8-08 logs mine should have updated. |
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.
Seems to work as far as I can tell!
accountsChanged
events stop after a dapp connection is closed.