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

Cannot click links in .jp-no-solution #179

Closed
robballou opened this issue Nov 12, 2013 · 6 comments
Closed

Cannot click links in .jp-no-solution #179

robballou opened this issue Nov 12, 2013 · 6 comments

Comments

@robballou
Copy link

When you try to access a link in the jp-no-solution div, you can't actually click the links.

To Replicate

  1. Disable Flash if you have it enabled.
  2. Go here: http://jplayer.org/latest/demo-01-supplied-mp3/
  3. You will see "Update Required To play the media you will need to either update your browser to a recent version or update your Flash plugin."
  4. Click on the "Flash plugin" link
@happyworm
Copy link
Contributor

I was just taking a look at this... Because it certainly used to work, at the very least when i first added it and then again when it was reviewed after changing the CSS to accommodate Popcorn's Subtitle plugin. But reviewing now on Firefox 25.0.1 on Win 7 and using Firebug to turn off the 2 display:none rules applied to the .jp-no-solution element, I could not get it working even all the way back to jPlayer 2.1.0.

Based on your original post, I am guessing that you are on OSX (or ubuntu) and using Firefox. I think in OSX Firefox 26 will start using the operating system codecs, but that is a tangent.

I will expand my review to include other browsers and post back again later.

@happyworm
Copy link
Contributor

After investigating, it is was found that the click handlers for the cssSelector system are suppressing the link effect.

In jPlayer 2.5.0 it looks like:

var handler = function(e) {
  e.preventDefault();
  self[fn](e);
  $(this).blur();
};

In jPlayer 2.3 and before, it used return false instead, which prevents default and also propagation.

The problem here is e.preventDefault() since the link to Flash is bubbling up the DOM and gets captured by this generic handler. In practice, the no-solution element does not need a handler, but it is the generic nature of the code that assumes that all cssSelectors have a click handler put on them.

I did have this item listed for my 2.5.0 changes, but it did not make the cut... Well, it was listed as:

  • cssSelectors should not require an empty internal function.

I had been planning to insert a conditional so that if the internal method did not exist, the handler would not be created... So indirectly I'd have fixed this problem too.

I was also mulling over changed to clicks on the time displays changing their format, such as timeLeft and now I'm thinking that while looking into this, I should review the jPlayer click event so that it occurs with clicks on the GUI. Wandering off topic though.

@happyworm
Copy link
Contributor

Oh and this must never have worked... It can't have.

@happyworm
Copy link
Contributor

That appears to fix the problem and also allows us to remove all the empty functions that were required for the GUI elements that did not use an action associated with clicking them.

I held off adding it, but want to note it here... But I considered also adding something along the lines of:

var handler = function(e) {
  if(e.target === this) {
    e.preventDefault();
    self[fn](e);
    $(this).blur();
  }
};

The idea being that the handler would ignore clicks on children... But that would have corrupted the way the bars works and the playBar() would have had to pass the event to the seekBar() method. ie., Instead of deleting the playBar() method like I just did in the last commit to the dev branch.

Hmm... This is really a bug fix... But I'll let this one slide.

Closing the issue.

Thank you for reporting it.

@happyworm
Copy link
Contributor

Did it properly after all... This is tagged as bug fix 2.5.1

@robballou
Copy link
Author

I'm glad you could figure it!

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

No branches or pull requests

1 participant