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

F/vl 547 captions firefox #12

Open
wants to merge 2 commits into
base: aws/develop
Choose a base branch
from

Conversation

JamesUoM
Copy link

@JamesUoM JamesUoM commented Oct 5, 2023

Allow caption selection in Firefox. This reverts a change that was made to aid accessibility as part of PAEL-8

To aid testing captions added to test pages

@JamesUoM JamesUoM added the bug Something isn't working label Oct 5, 2023
@JamesUoM JamesUoM requested review from ppettit and tyfranck October 5, 2023 13:31
Copy link

@ppettit ppettit left a comment

Choose a reason for hiding this comment

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

tested on latest chrome, firefox and safari and seems to work fine 👍

Copy link

@ppettit ppettit left a comment

Choose a reason for hiding this comment

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

I took another peek at this and I think there is a bug stopping the popup from being closed in the PopupContainer base class:

$(button).keypress(function(event) {
if ( (event.keyCode == 13) && (!this.plugin.isPopUpOpen()) ){
if (this.plugin.isPopUpOpen()) {
paella.player.controls.playbackControl().hidePopUp(this.popUpIdentifier,this,true);
}
else {
paella.player.controls.playbackControl().showPopUp(this.popUpIdentifier,this,true);
}
}
else if ( (event.keyCode == 27)){
paella.player.controls.playbackControl().hidePopUp(this.popUpIdentifier,this,true);
}
event.preventDefault();
});

It definitely seems like a bug as it makes the hidePopup on line 357 unreachable. The following change on line 356 fixes it:

-                        if ( (event.keyCode == 13) && (!this.plugin.isPopUpOpen()) ){
+                        if (event.keyCode == 13)  {

The code does also check for ESC key, but (at least for me on linux/firefox) ESC events never get to this bit of code for some reason.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants