Skip to content

fix: Determine the higher version of WeChat browser for Windows#5913

Merged
dylans merged 2 commits intoianstormtaylor:mainfrom
LLwill:fix/wechat-browser-env
Jul 10, 2025
Merged

fix: Determine the higher version of WeChat browser for Windows#5913
dylans merged 2 commits intoianstormtaylor:mainfrom
LLwill:fix/wechat-browser-env

Conversation

@LLwill
Copy link
Contributor

@LLwill LLwill commented Jul 5, 2025

Description
Now the latest WeChat browser for Windows has used a newer version of Chrome

Issue
Fixes: https://github.com/ianstormtaylor/slate/issues/5912

!/.*MacWechat/.test(navigator.userAgent) // avoid lookbehind (buggy in safari < 16.4)

!/.*MacWechat/.test(navigator.userAgent) && // avoid lookbehind (buggy in safari < 16.4)
(IS_CHROME ? IS_CHROME_LEGACY : true) // wechat and low chrome is real wechat
Copy link
Contributor

@12joan 12joan Jul 5, 2025

Choose a reason for hiding this comment

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

This is equivalent to !IS_CHROME || IS_CHROME_LEGACY. If that's what you intended, can we replace the condition with this?

Is it possible for the !IS_CHROME case to arise if the user agent contains Wechat (i.e. are there WeChat versions that don't mention Chrome in their user agents)? If not, we can simplify to IS_CHROME_LEGACY.

You said in the PR description:

Now the latest WeChat browser for Windows has used a newer version of Chrome

So it sounds like IS_WECHATBROWSER shouldn't be dependent on IS_CHROME_LEGACY. Do we actually want to rename this constant IS_WECHAT_LEGACY?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main concern is that the old version still uses the lower version of the Chrome kernel, so I added a bottom-line judgment. In fact, it is not necessary for the latest version at present.
Now UA:

Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126.0.0.0 Safari/537.36 NetType/WIFI MicroMessenger/7.0.20.1781(0x6700143B) WindowsWechat(0x63090c33) XWEB/13639 Flue

Copy link
Contributor

Choose a reason for hiding this comment

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

That makes sense. It sounds like the best thing to do would be to rename the constant to IS_WECHAT_LEGACY to clarify that special behaviour is only needed on the old version, and to simplify the boolean condition.

If the old version's user agent always contains the string Chrome, then && IS_CHROME_LEGACY should be sufficient; no need to check IS_CHROME.

Otherwise, I would recommend && (!IS_CHROME || IS_CHROME_LEGACY).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, this looks more elegant, I will modify it.

@changeset-bot
Copy link

changeset-bot bot commented Jul 7, 2025

⚠️ No Changeset found

Latest commit: d7b70ee

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@dylans dylans merged commit 5fdd83e into ianstormtaylor:main Jul 10, 2025
10 checks passed
@12joan
Copy link
Contributor

12joan commented Jul 11, 2025

@dylans This was merged without a changeset. One will need to be added before #5917 is merged to ensure slate-dom gets released.

@12joan 12joan mentioned this pull request Jul 11, 2025
12joan added a commit to 12joan/slate that referenced this pull request Jul 11, 2025
@12joan 12joan mentioned this pull request Jul 11, 2025
dylans pushed a commit that referenced this pull request Jul 12, 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.

3 participants