Skip to content

Covert BrowserDetection to TS#69

Merged
jallamsetty1 merged 9 commits intojitsi:masterfrom
jallamsetty1:master
Jun 3, 2025
Merged

Covert BrowserDetection to TS#69
jallamsetty1 merged 9 commits intojitsi:masterfrom
jallamsetty1:master

Conversation

@jallamsetty1
Copy link
Member

Remove BrowserCapabilities class since lib-jitsi-meet no longer uses it.

Remove BrowserCapabilities class since lib-jitsi-meet no longer uses it.
@jallamsetty1 jallamsetty1 requested a review from saghul August 13, 2024 20:30
* the current browser version is unknown.
*/
isVersionGreaterThan(version) {
isVersionGreaterThan(version: number): boolean | undefined {
Copy link
Member

Choose a reason for hiding this comment

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

Let's make this return a boolean always. If the version is not available, return false.

* the current browser version is unknown.
*/
isVersionLessThan(version) {
isVersionLessThan(version: number): boolean | undefined {
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

* A loose-equality operator is used here so that it matches the sub-versions as well.
*/
isVersionEqualTo(version) {
isVersionEqualTo(version: number): boolean | undefined {
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

* the current engine version is unknown.
*/
isEngineVersionGreaterThan(version) {
isEngineVersionGreaterThan(version: number): boolean | undefined {
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

* the current engine version is unknown.
*/
isEngineVersionLessThan(version) {
isEngineVersionLessThan(version: number): boolean | undefined {
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

* A loose-equality operator is used here so that it matches the sub-versions as well.
*/
isEngineVersionEqualTo(version) {
isEngineVersionEqualTo(version: number): boolean | undefined {
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@@ -0,0 +1,36 @@

// TODO: Maybe fix the values to 'Chrome', 'Internet Explorer', etc. Currently
Copy link
Member

Choose a reason for hiding this comment

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

I guess we can drop this TODO?

* Maps the names of the browsers from ua-parser to the internal names defined in
* ./browsers.js
*/
export const PARSER_TO_JITSI_NAME: { [key: string]: Browser } = {
Copy link
Member

Choose a reason for hiding this comment

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

How do we parse Chromium engine browsers like Edge or Brave?

Copy link
Member Author

@jallamsetty1 jallamsetty1 May 30, 2025

Choose a reason for hiding this comment

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

Chromium based browsers like Brave, Edge and Vivaldi are detected as Chrome and Chromium based since they all have the same blink engine. That suffices since we treat them as Chrome functionality wise.
https://github.com/jitsi/js-utils/pull/69/files#diff-a805b39c7a52d0c0d5d8f31b025be8409294f04623d6384493fcbaf498620b18R170

package.json Outdated
{
"name": "@jitsi/js-utils",
"version": "2.2.1",
"version": "2.2.2",
Copy link
Member

Choose a reason for hiding this comment

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

The auto-publisher ran because you used your master branch. Can you back out the version change?

Copy link
Member

Choose a reason for hiding this comment

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

Pl undo this.

@jallamsetty1 jallamsetty1 force-pushed the master branch 2 times, most recently from 21bfd52 to 094613a Compare May 30, 2025 17:58
@jallamsetty1 jallamsetty1 requested a review from saghul May 30, 2025 18:00
Copy link
Member

@saghul saghul left a comment

Choose a reason for hiding this comment

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

LGTM with a comment.

@jallamsetty1 jallamsetty1 merged commit 21aa0c9 into jitsi:master Jun 3, 2025
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.

2 participants