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

infinite loop for unprefixed use. #2675

Closed
jdalton opened this issue Oct 5, 2015 · 6 comments
Closed

infinite loop for unprefixed use. #2675

jdalton opened this issue Oct 5, 2015 · 6 comments
Labels

Comments

@jdalton
Copy link

jdalton commented Oct 5, 2015

I noticed in src/js/player.js that if fsApi.fullscreenchange is unprefixed, so fullscreenchange, then it will trigger an infinite loop.

Events.on(document, fsApi.fullscreenchange, Fn.bind(this, function documentFullscreenChange(e){
  this.isFullscreen(document[fsApi.fullscreenElement]);

  // If cancelling fullscreen, remove event listener.
  if (this.isFullscreen() === false) {
    Events.off(document, fsApi.fullscreenchange, documentFullscreenChange);
  }
  this.trigger('fullscreenchange'); // <-- ohh noo
}));
@heff
Copy link
Member

heff commented Oct 5, 2015

Thanks for the report! The listener is on the document but the context of the documentFullscreenChange function should be the player, so I'm not totally following how this is causing the loop yet, unless the event is bubbling up to the document from the player. Looking into it more.

@heff heff added the bug label Oct 5, 2015
@jdalton
Copy link
Author

jdalton commented Oct 5, 2015

Actually this is the issue. Do you have any info on the commit that resolved?

@jdalton jdalton closed this as completed Oct 5, 2015
@heff
Copy link
Member

heff commented Oct 5, 2015

Ah, yeah.
#2351
07f0483

So if you're using 5.0 (not an RC) I would expect this to be fixed.

@jdalton
Copy link
Author

jdalton commented Oct 5, 2015

Rock! Is it tricky to induce bubbling? (curious if you're using a real event or fake one )

@heff
Copy link
Member

heff commented Oct 5, 2015

Mostly fake events that are copied from real ones . Built off the JSNinja example years ago and improved along the way.

You can set event.bubbles = true and it will bubble up. That should work with native events that bubble automatically, though I'm starting to see some weird behavior with keyboard events.

@riking
Copy link

riking commented Sep 4, 2018

There are still pages in the wild hitting this bug, as per webcompat/web-bugs#2498 , such as mentioned in this comment. Would you consider releasing a hot-fix for the old versions on your CDN or working with the affected sites to update the library?

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants