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

Older browser versions querySelector error #391

Closed
1 of 4 tasks
hhatakeyama opened this issue Jan 13, 2023 · 7 comments · Fixed by #408
Closed
1 of 4 tasks

Older browser versions querySelector error #391

hhatakeyama opened this issue Jan 13, 2023 · 7 comments · Fixed by #408
Assignees
Labels

Comments

@hhatakeyama
Copy link

Issue Type

  • Bug report
  • Feature request
  • Question / support request
  • Other

Description

On older chrome versions (tested on v86.0) and samsung internet browser (tested on v13), the following error was reported on sentry:

Failed to execute 'querySelector' on 'Element': ':is(img, svg, [role="img"], [data-zoom]):not([aria-hidden="true"])' is not a valid selector.

image

@rpearce
Copy link
Owner

rpearce commented Jan 13, 2023

Hi @hhatakeyama, and thank you for opening your issue.

Chrome 86 looks like it came out over 2 years ago, and the global usage of it is very small. For Samsung browser v13, it is even less.

Before I go about making any changes, I have a few questions:

  • Can your Chrome browsers be updated?
  • For Chrome, it looks like it might work if a flag is enabled by the user. Can they do this? Support for :is() can be enabled with the Experimental Web Platform features flag
  • How critical is this tool for you? I am concerned that if you're supporting lots of old browsers, this may just be 1 of potentially a number of issues with using newer browser features. If it's blocking you, then could you have a conditional where you check for browser support for this :is and, if it's not supported, you only render a regular image?

Thanks for your time!

@hhatakeyama
Copy link
Author

hhatakeyama commented Jan 13, 2023

Hey @rpearce thanks for the reply.
We're using your plugin for our ecommerce and the problem is that a lot of clients are complaining about this issue, and we don't have the contact of all of them, but we're looking forward to a polyfill fix, since we've found out there's other plugins that have the same issue.

Thanks for your attention, and great work!

@rpearce
Copy link
Owner

rpearce commented Jan 13, 2023

@hhatakeyama Ok, I will look into this.

@rpearce
Copy link
Owner

rpearce commented Mar 20, 2023

I'm sorry I haven't circled back to this. Other things have taken priority. Is this still an issue for you, @hhatakeyama?

@pacocoursey
Copy link

I'm still encountering this error frequently. A possible solution could be to use the CSS.supports() API to check for :is() support, and fallback to the :-webkit-any() selector otherwise. This would increase support back to Chrome 15 and Safari 5.1, according to caniuse.

@rpearce
Copy link
Owner

rpearce commented Apr 16, 2023

@pacocoursey @hhatakeyama Is this what that would basically look like?

let selector = `:is(img, svg, [role="img"], [data-zoom]):not([aria-hidden="true"])`

if (!CSS.supports(selector)) {
  selector = `img:not([aria-hidden="true"]), svg:not([aria-hidden="true"]), [data-zoom]:not([aria-hidden="true"])`
}

At which point maybe it's just better to do this...

const selector = ['img', 'svg', '[data-zoom]']
  .map(x => `${x}:not([aria-hidden="true"])`)
  .join(',')

document.querySelector(selector)

What do y'all think?


edit: How does this PR look to you (#402)? If you like the approach, I can publish a release candidate, have you test it out, and if it's all good for you, we can publish it fully.

@rpearce
Copy link
Owner

rpearce commented Apr 16, 2023

Can y'all please 🙏 try this fix out? npm i react-medium-image-zoom@5.1.4-rc.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants