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

Firefox 65 table scroll by keyboard #9623

Open
mappopo opened this issue Feb 5, 2019 · 18 comments

Comments

Projects
None yet
8 participants
@mappopo
Copy link

commented Feb 5, 2019

In Firefox 65 is no longer possible to change the row selection by keyboard.

The behaviour can be replicated in playgroud http://www.qooxdoo.org/current/demobrowser/#table~Table.html
Select a row with a mouse click and try use keyboard arrows up and down to move selection. It takes no effect. With other browser (eg. Edge or Chrome) the focus and selection successful changes.

@zaucker

This comment has been minimized.

Copy link
Member

commented Feb 5, 2019

@cboulanger

This comment has been minimized.

Copy link
Contributor

commented Feb 5, 2019

Works on MacOS 10.14.2, FF 65.

@mappopo

This comment has been minimized.

Copy link
Author

commented Feb 5, 2019

I use windows, in devel playgroun is ok. Is it safe use devel version?

@hkollmann

This comment has been minimized.

Copy link
Member

commented Feb 5, 2019

Yes it is. Most of us use git repos version

@cboulanger

This comment has been minimized.

Copy link
Contributor

commented Feb 5, 2019

I see - I can confirm, http://www.qooxdoo.org/current/demobrowser/#table~Table.html is broken for Firefox 65 on the Mac. Looks like we need a backport of the fix?

@level420

This comment has been minimized.

Copy link
Member

commented Feb 5, 2019

@mappopo did it work with firefox 64?

@lucapivato

This comment has been minimized.

Copy link
Contributor

commented Feb 5, 2019

https://www.fxsitecompat.com/en-CA/docs/2018/non-printable-keys-no-longer-fire-keypress-event/

An exception is the Enter key; pressing Enter with no modifiers, Shift+Enter or Ctrl+Enter will fire the keypress event as before, which is invalid in the current spec but compatible with other browsers.

This genius change may be coming to Chrome too...

Actually it's like that in Chrome.

@level420

This comment has been minimized.

Copy link
Member

commented Feb 5, 2019

@lucapivato yes, currently investigating that and why it still works in master.

@level420

This comment has been minimized.

Copy link
Member

commented Feb 5, 2019

The fix is PR #216
@mappopo could you please try the fix in the PR above and report back? Thank you!

@level420

This comment has been minimized.

Copy link
Member

commented Feb 5, 2019

created #9624 for branch_5_0_x with cherry picked PRs #216 and #9404 from master.

@mappopo

This comment has been minimized.

Copy link
Author

commented Feb 6, 2019

The fix is PR #216
@mappopo could you please try the fix in the PR above and report back? Thank you!

it works, thank you

@adrelino

This comment has been minimized.

Copy link
Contributor

commented Feb 9, 2019

In Firefox' 65 released on January 29th 2019, my legacy web app built on top of qooxdoo doesn't work properly anymore in many places since I haven't been maintaining it (especially with regard to events) properly.

Is it save to just replace keypress with keydown and _onKeyPress with _onKeyDown (used e.g. in our own Table subclass) everywhere in my own code which extends qooxdoo?

Even though keypress still works in chrome, it seems to be officially deprecated now (https://developer.mozilla.org/en-US/docs/Web/Events/keypress, https://w3c.github.io/uievents/#event-type-keypress) and even Mozilla suggests to replace it with beforeinput or keydown. Or are there any downsides expected from this?

@adrelino

This comment has been minimized.

Copy link
Contributor

commented Feb 9, 2019

While searching for "keypress", I just stumbled upon another related bug inside qooxdoo code.

For the same reason, even in the current devel version, one cannot close an opened SelectBox anymore by pressing Escape in Firefox 65, while in Chrome 71 it works:

http://www.qooxdoo.org/devel/demobrowser/#widget~SelectBox.html

The reason is that AbstractSelectBox is still listening for keypress instead of keydown.

// Register listeners
this.addListener("keypress", this._onKeyPress);
this.addListener("blur", this._onBlur, this);

source

@adrelino

This comment has been minimized.

Copy link
Contributor

commented Feb 9, 2019

Yet another related issue:
Controlling a RadioButton with the arrow keys works in chrome 72, while in Firefox 65:

  • doesn't work in current
  • whereas in devel, one arrow key press triggers moving to the next radio item 2 times (so you get 2 alerts) and end up 2 options further...
@level420

This comment has been minimized.

Copy link
Member

commented Feb 12, 2019

@adrelino Thank you for reporting this. From Firefox 65 on there's no keypress event sent anymore for non printable characters. See https://www.fxsitecompat.com/en-CA/docs/2018/non-printable-keys-no-longer-fire-keypress-event/

The solution is indeed to switch over to use the keydown event in this situation.

It would be great if you can offer a PR for master which I'll then backport, if applicable, to branch 5_0_x.

I think generating artificial keypress events for browsers not sending native keypress anymore isn't an option, as it seems that browsers will in the end follow step by step the specs and deprecate keypress (https://w3c.github.io/uievents/#keypress)

@oetiker

This comment has been minimized.

Copy link
Member

commented Feb 12, 2019

I wonder if we could put something in place to warn authors about this problem ...maybe have the compailer complain if it sees keypress eventhandlers being assigned ? @johnspackman

@level420

This comment has been minimized.

Copy link
Member

commented Feb 12, 2019

@oetiker I think we should switch over to #9626 with our discussion

@level420

This comment has been minimized.

Copy link
Member

commented Feb 15, 2019

@adrelino the problem with not closing the select box popup via escape in FF 65 should be solved with PR #9629

The problem of the double event when navigating in radio buttons in the demo http://cpr2/qooxdoo-level420/application/demobrowser/source/#widget~RadioButton.html is still there, but it may be another problem where the event somehow interacts with the alert popup displayed on value change. There is no problem in navigating with the arrow keys in the demo http://cpr2/qooxdoo-level420/application/demobrowser/source/#widget~RadioButtonGroup.html where no alert box is displayed on value change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.