Skip to content
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

feat: don't autoblock non-chrome browsers #2096

Merged

Conversation

filipesilva
Copy link
Collaborator

@filipesilva filipesilva commented Mar 27, 2022

Close #1718
Close #1705

I tried this PR on firefox and it seems to open ok, so I guess the case problem in those issues went away.

@vercel
Copy link

vercel bot commented Mar 27, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/athens-research/athens/5fj9LuzZZUMXUwR1qFMh9VtgHV1o
✅ Preview: https://athens-git-fork-filipesilva-remove-unsup-ba422d-athens-research.vercel.app

@filipesilva
Copy link
Collaborator Author

Best reviewed with the hide whitespace option.

@tangjeff0
Copy link
Collaborator

Browsers

  • Brave
  • Chrome
  • Edge
  • Firefox

OSes

  • Windows
  • Linux
  • Mac

@filipesilva
Copy link
Collaborator Author

filipesilva commented Mar 30, 2022

I tested on Mac with chrome/brave/edge/firefox and they seem to work.

Safari on mac does not work, neither does iOS Safari or Chrome (same engine). Safari on mac throws this error:

SyntaxError: Invalid regular expression: invalid group specifier name

I traced it down to some lookbehinds we have on regexes (both ?<= and ?<!), which aren't supported on Safari yet (https://caniuse.com/?search=lookbehind, #2096, https://bugs.webkit.org/show_bug.cgi?id=174931).

We have a lot of lookbehinds in our parser so that's definitely not easy to refactor, and I can see in Safari over a dev build that these are a problem:
image

@vercel
Copy link

vercel bot commented Apr 20, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
athens ✅ Ready (Inspect) Visit Preview Apr 20, 2022 at 10:47AM (UTC)

function isBrowserSupported() {
return userAgent.indexOf("chrome/") > -1;
function isBrowserUnsupported() {
return userAgent.indexOf("applewebkit/") > -1;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This isn't right, the user agent for chrome under macos is 'mozilla/5.0 (macintosh; intel mac os x 10_15_7) applewebkit/537.36 (khtml, like gecko) chrome/100.0.4896.75 safari/537.36'

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Brave is also the above.

Firefox is "mozilla/5.0 (macintosh; intel mac os x 10.15; rv:98.0) gecko/20100101 firefox/98.0".

Safari is "mozilla/5.0 (macintosh; intel mac os x 10_15_7) applewebkit/605.1.15 (khtml, like gecko) version/15.4 safari/605.1.15"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've updated this check and can verify it works as follows:

MacOS:

  • Safari shows the Safari based browsers based are not supported message
  • Chrome, Firefox and Brave don't

iOS:

  • All browsers show the Safari based browsers based are not supported message.

@filipesilva filipesilva merged commit 4717a1d into athensresearch:main Apr 20, 2022
@filipesilva filipesilva deleted the remove-unsupported-browsers branch April 20, 2022 10:59
@waldyrious
Copy link

Just for the record, I confirmed the demo site now works on Ubuntu in both Firefox v99.0 and Brave v1.37.116 (Chromium 100.0.4896.127).

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.

Demosite down Demo linked to in github sidebar doesn't work for me.
3 participants