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

Fixes issues with firefox and safari. #1058

Merged
merged 12 commits into from
Apr 4, 2024
Merged

Fixes issues with firefox and safari. #1058

merged 12 commits into from
Apr 4, 2024

Conversation

zorkow
Copy link
Member

@zorkow zorkow commented Feb 15, 2024

These are the workarounds for the issues regarding voice events on Firefox.
@dpvc can you have a look if this works for Safari as well?

@zorkow zorkow requested a review from dpvc February 15, 2024 04:41
Copy link
Member

@dpvc dpvc left a comment

Choose a reason for hiding this comment

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

This is still not working with Firefox, but that seems to be due to the fact that the role attributes are missing. I think you did something in SRE to fix this, but I don't have that updated SRE, so my Firefox isn't working.

Safari is nearly working. I can click to activate the explorer, and use keyboard to move around the equation, and the speech and braille show up properly. But if the explorer is active, then clicking does not select the clicked item. I tracked this down in the code, and the difference between Safari and Chrome is in how this line

prev.removeAttribute('tabindex');

is handled. In Safari (and presumably Firefox for you), when the tabindex is removed, the browser fires a focus-out even immediately, and the KeyExplorer's FocusOut method is called immediately before the rest of the Click handler is run. In particular, that means that the Stop() method will be run before the rest of the Click method, and so this.active will be set to false before this.Start() is run at line 184.

In Safari, moving the tabindex does not fire a focus-out event, so this.active is still true when this.Start() is called, and because Start() returns if this.active is true on this line

if (this.active) return;

in Safari, the rest of the Start() doesn't run, and the selected node is never switched.

One solution would be to add

this.FocusOut(null);

right after line 180 shown above. That will force the focus-out action in Safari, and for browsers that fire a focus-out event at this point, it should still be OK since FocusOut() exists if this.active is false, as it should be after the first FocusOut() is run. Of course, you might have a better idea for this.

I'll have to check Firefox again once I have the SRE update that allows the role attributes to be inserted.

@dpvc dpvc added this to the v4.0 milestone Feb 17, 2024
@dpvc
Copy link
Member

dpvc commented Feb 19, 2024

Here are the list of continuing issues from our chat today:

  • Hover highlighting leads to an error.
  • While explorer is active, all key events are blocked (including reload, etc.)
  • Can't start explorer if both speech and braille are unchecked. I believe that this is due to the fact that there are no role attributes in that case, because those seem to be added during the speech/braille creationSpeech() function in semantic-enrich, which isn't being called.
  • After a change in language, you must do a move to get new language (but you know this one already).

@dpvc
Copy link
Member

dpvc commented Feb 21, 2024

I figured out the third issue above, and have fixed it in my menu update branch (not yet pushed).

@zorkow
Copy link
Member Author

zorkow commented Mar 6, 2024

Here are the list of continuing issues from our chat today:

  • Hover highlighting leads to an error.

This was a CHTML error in SRE, which is now being caught (latest develop branch). But note that hover only works when the explorer is active. Otherwise we do not have a highlighter as those are only created when we have an ExplorerItem. We can probably change that in the future, but I am not sure if it is worth it.

  • While explorer is active, all key events are blocked (including reload, etc.)

That is now no longer the case.

  • Can't start explorer if both speech and braille are unchecked. I believe that this is due to the fact that there are no role attributes in that case, because those seem to be added during the speech/braille creationSpeech() function in semantic-enrich, which isn't being called.

That was actually an issue related to how explorers were attached. Previously, speech, braille and keyMagnifier would attach their own explorer, now only speech attaches the SpeechExplorer which handles all three duties. So we need to see if at least one of the three options in options.a11y is set, to attach and thus enable the SpeechExplorer.

  • After a change in language, you must do a move to get new language (but you know this one already).

We now have a similar issue with switching speech/braille before retypesettting. This is best fixed once the new Menu structure is integrated.

@dpvc
Copy link
Member

dpvc commented Mar 6, 2024

That was actually an issue related to how explorers were attached.

I fixed it in the sre-menus branch, as I mentioned above, so you don't need to worry about that one.

Base automatically changed from feature/better_honking to develop March 7, 2024 14:04
@zorkow
Copy link
Member Author

zorkow commented Mar 28, 2024

After a change in language, you must do a move to get new language (but you know this one already).

This is now fixed. However, we still have an issue when changing rule sets/preference settings. I will tackle this in a new PR to simplify review.

@zorkow
Copy link
Member Author

zorkow commented Mar 28, 2024

@dpvc as discussed today, this should be ready to merge.

Copy link
Member

@dpvc dpvc left a comment

Choose a reason for hiding this comment

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

This all seems to work now, except that I still can't get hover or flame highlighting to work. But I may not know what I'm doing, so that may be the problem. We can chat about it in our next meeting.

@zorkow zorkow merged commit da33c5d into develop Apr 4, 2024
@zorkow zorkow deleted the fix/ff_safari_issues branch April 4, 2024 12:46
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

Successfully merging this pull request may close these issues.

2 participants