Skip to content

fix(analytics): detect browser for Chromium-based browsers - #2648

Open
creeperkatze wants to merge 6 commits into
wxt-dev:mainfrom
creeperkatze:fix/analytics-browser-detection
Open

creeperkatze wants to merge 6 commits into
wxt-dev:mainfrom
creeperkatze:fix/analytics-browser-detection

Conversation

@creeperkatze

Copy link
Copy Markdown
Contributor

Overview

The browser user property was always empty for Chromium-based browsers when built, since ua-parser-js couldn't read the user agent in MV3 service workers. It also couldn't tell browsers like Brave apart from Chrome.

This PR replaces it with bowser, which also reads client hints. browser now reports IDs like chrome, edge, brave and firefox.

Also:

  • Removed the optimizeDeps.include entry for ua-parser-js.
  • Removed ua-parser-js from the excluded packages in upgrade-deps.ts.
  • The Moderok provider now maps Brave, Opera, Vivaldi, Yandex and Naver Whale to other_chromium instead of unknown.

Manual Testing

  1. Run bun run dev:build in packages/analytics and load .output/chrome-mv3 in Chrome and Edge.
  2. Open the popup and click a button.
  3. In the background's console, the logged track event should show user.properties.browser as chrome / edge. Before this change it was undefined.

@netlify

netlify Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for creative-fairy-df92c4 ready!

Name Link
🔨 Latest commit b4bc33a
🔍 Latest deploy log https://app.netlify.com/projects/creative-fairy-df92c4/deploys/6acb7d7fe0c1ac000809e23f
😎 Deploy Preview https://deploy-preview-2648--creative-fairy-df92c4.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 04a9a7d8-c50c-477d-9e95-e3b5a9613acb

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 579fb38d-6883-4f01-9f9d-541afcd770ad



📥 Commits

Reviewing files that changed from the base of the PR and between 6ee6031 and b8f29ce.




⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock



📒 Files selected for processing (5)
  • packages/analytics/modules/analytics/client.ts
  • packages/analytics/modules/analytics/index.ts
  • packages/analytics/modules/analytics/providers/moderok.ts
  • packages/analytics/package.json
  • scripts/upgrade-deps.ts



💤 Files with no reviewable changes (2)
  • packages/analytics/modules/analytics/index.ts
  • scripts/upgrade-deps.ts



Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.





📝 Walkthrough
📝 Walkthrough
📝 Walkthrough

Walkthrough

The analytics client replaces ua-parser-js with Bowser, uses browser name and version data in analytics properties, and adds mappings for five browsers. Dependency preprocessing and upgrade-script filtering also change.

Changes

Analytics browser detection

Layer / File(s) Summary
Browser detection and dependency update
packages/analytics/modules/analytics/client.ts, packages/analytics/modules/analytics/providers/moderok.ts, packages/analytics/modules/analytics/index.ts, packages/analytics/package.json, scripts/upgrade-deps.ts
The client detects browsers with Bowser, including optional userAgentData, and uses the detected name and version in analytics properties. Brave, Opera, Vivaldi, Yandex, and Naver map to other_chromium. The package replaces ua-parser-js with bowser; Vite preprocessing and the dependency ignore-list entry for ua-parser-js are removed.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix





Merge Risk: ⚪ Minimal · up to b8f29

Browser metadata now uses Bowser, and additional Chromium browsers map to other_chromium. The possible Brave misclassification depends on a missing hint in the MV3 background context, which was not established as reachable; no verified issue warrants blocking merge.

Security Architecture Review

Security architecture risk: 🔵 Low · up to b8f29

The change remains within browser identification and preserves the existing analytics upload controls and destinations. No introduced security concern was established, but browser-runtime behavior and some dependency-related changes remain only partially verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated exposure change is browser-metadata classification for analytics users. In the inspected flow, those values do not select provider destinations or confer authority; provider endpoint selection and sending paths are unchanged.

Trust Boundaries and Controls

  • observed — Client hints enter through the existing background factory and are reduced to browser ID and version before event construction. The inspected event properties do not include raw userAgentData.
  • observed — The existing named runtime-port listener dynamically dispatches analytics methods, and frontend forwarding remains unchanged. That listener does not visibly authenticate the sender beyond matching the port name, but the comparison establishes no PR-induced expansion of this boundary; broader caller provenance remains unverified.
  • observed — Provider uploads remain guarded by the existing enabled check. Event construction and optional debug logging occur before that check, as they did before this PR.



🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: detecting Chromium-based browsers in analytics.
Description check ✅ Passed The description includes an overview and concrete manual testing steps. It omits the Related Issue section, but the remaining content is complete and relevant.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.



Full details: Docstring Coverage

Explanation

Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (1 skipped: 1 unsupported.)






✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR





  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the pkg/analytics Includes changes to the `packages/analytics` directory label Oct 4, 2026
@creeperkatze

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Pull request base or head changed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@creeperkatze

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@PatrykKuniczak

Copy link
Copy Markdown
Collaborator

@creeperkatze Hi

I've been trying to reproduce this undefined but i really can't.
Let's add more repro steps for previous and current approach.

I think button in popup should call .track() from moderok.
Maybe create another branch on your fork and then let's make example and let me know.
I'll pull it on my machine.

It looks interesting, but i need to test it manually before any approval.

And resolve conflict with bun.lock file.

Thanks

@creeperkatze

Copy link
Copy Markdown
Contributor Author

Thanks! I pushed the branch repro/browser-detection on my fork. It adds a "Track event" button to the analytics demo popup. The first commit is just the button on top of main, and the second merges this PR, so you can compare both.

  1. Check out a commit, then run bun install and bun run dev:build in packages/analytics
  2. Load .output/chrome-mv3 in Chrome or Edge
  3. Click "Track event" in the popup
  4. In the service worker console (not the popup's), check user.properties in the [@wxt-dev/analytics] track log

Before, browser is missing. After, it's chrome / edge. Firefox isn't affected.

@creeperkatze

Copy link
Copy Markdown
Contributor Author

Also resolved the bun.lock conflict!

Comment on lines 27 to 36
chrome: 'chrome',
edge: 'edge',
firefox: 'firefox',
chromium: 'other_chromium',
brave: 'other_chromium',
opera: 'other_chromium',
vivaldi: 'other_chromium',
yandex: 'other_chromium',
naver: 'other_chromium',
};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@creeperkatze Thanks a lot, now i see difference.

But there's unknown for safari, because it isn't listed there.

Another problem is all browsers below chromium is other_chromium, IMO this should be named 'opera': 'opera' and etc.

But i've been wondering if it's possible and good solution to use Bowser.BROWSER_MAP there, instead of our BROWSER_MAP, what do you think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Glad you could reproduce it!

I kept Moderok's values in line with their own SDK, which only sends chrome, edge, firefox, other_chromium and unknown (Safari and others end up as unknown). Not sure if their API accepts more than that, if it does, happy to pass the browser through directly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As for Bowser.BROWSER_MAP, it just maps Bowser's IDs to display names (e.g. edge becomes "Microsoft Edge"). Moderok still needs its own values.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg/analytics Includes changes to the `packages/analytics` directory

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants