lhz516 / react-h5-audio-player

React audio player component with UI. It provides time indicator on both desktop and mobile devices.
https://codepen.io/lhz516/pen/dyGpmgP
MIT License
608 stars 92 forks source link

Clicking on the ProgressBar throws an error when using Audio player within an iframe #201

Open ccpu opened 1 year ago

ccpu commented 1 year ago

Describe the bug

There is a problem with the progress bar when loading audio player within an Iframe. As it turns out, "getPosX" does not use the mouse event, but instead attempts to use the touch event.

capture

Step to reproduce codesandbox

Environment

Package version: 3.8.6 React version: 17.0.16 Browser and its version: Chrome OS and its version: win10

lhz516 commented 1 year ago

Thanks for reporting. iframe hans't been considered. Let me take a look.

lhz516 commented 1 year ago

@ccpu I think it's the issue of react-frame-component. See the console output below. I set a debugger inside getPos function, the event is MouseEvent, but event instanceof MouseEvent is false. I guess react-frame-component uses its own copy of MouseEvent which is not the same as the browser native one. A real native iframe shouldn't have an issue. image

ccpu commented 1 year ago

Thank you for taking the time to look into this issue. It appears that it is not the react-frame-component package that is causing the problem, but rather Iframe itself. It could be a bug or security issue, but I am not sure.

Here is another example that directly uses Iframe: codesandbox

While I'm not certain what is the optimal solution for this problem, I think checking event.touches would be more beneficial:

export const getPosX = (event: TouchEvent | MouseEvent): number => {
  if (event.touches) {
    return event.touches[0].clientX
  } else {
    return event.clientX
  }
}

Is this an approach you might consider?

lhz516 commented 1 year ago

image TS complains the type issue on your proposed solution

I also cannot use if (event instanceof TouchEvent) because TouchEvent is undefined in some browsers. So it will be a bit tricky. If you have a good solution that can pass the build, feel free to create a PR

ccpu commented 1 year ago

I have assumed that you don't want to use the as keyword as a way of achieving it, so I have put it on my to-do list and will look into it in more depth when I get the chance.