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

Shortcuts also work in rich text editors #7156

Closed
jurmatix opened this issue Apr 5, 2016 · 7 comments · Fixed by #11253
Closed

Shortcuts also work in rich text editors #7156

jurmatix opened this issue Apr 5, 2016 · 7 comments · Fixed by #11253
Labels

Comments

@jurmatix
Copy link

jurmatix commented Apr 5, 2016

Configuration:

  • Web browser and its version: Chrome latest
  • Operating system and its version: Win10
  • PDF.js version: there is no version number in viewer.js :(
  • Is an extension: Yes. I've added an rich text editor to make annotations to the PDF

Steps to reproduce the problem:

  1. Adding RTE to viewer.html
  2. Pressing 'r', 'k', 'p', 'j', 'n' or any other shortcut without Shift or Control in RTE.

What is the expected behavior?
The pressed key should be only inserted in to the RTE.

What went wrong?
The shortcut will be also performed in pdf.js (rotating page while pressing 'r' ...).
So far this is the correct behaviour. But there is already code for exeptions starting in line 7823 in viewer.js. I.e. for input, textarea and select fields. But this won't work with div based rich text editors.
Would it make sense to check also "Element.attributes.contenteditable" so that the shortcuts won't get handled if the focus is in an editable element?

@yurydelendik
Copy link
Contributor

Would it make sense to check also "Element.attributes.contenteditable" so that the shortcuts won't get handled if the focus is in an editable element?

We do something similar at https://github.com/mozilla/pdf.js/blob/master/web/viewer.js#L2016 -- extend it to recognize other custom elements.

@Rob--W
Copy link
Member

Rob--W commented Apr 6, 2016

Add || curElement.isContentEditable to the block that yury referenced.

@chitgoks
Copy link

chitgoks commented Jun 8, 2016

Or the quickest workaround would be to have the element contenteditable key listener call event.stopPropagation()

@angularPublic
Copy link

angularPublic commented Jun 10, 2017

Adding the marked line below in pdf.js fixed the issue with textAngular key events.

Thanks Rob.

...
var curElement = document.activeElement || document.querySelector(':focus');

if(curElement.isContentEditable) return;

var curElementTagName = curElement && curElement.tagName.toUpperCase();
if (curElementTagName === 'INPUT' || curElementTagName === 'TEXTAREA' || curElementTagName === 'SELECT') {
if (evt.keyCode !== 27) {
return;
}
}
....

@Snuffleupagus
Copy link
Collaborator

Rather than leaving this open indefinitely, I'd suggest doing one of two things here:

  • Preferably closing the issue as WONTFIX, since this doesn't affect the default viewer as such (but rather only a custom implementation of it).
  • Given the simplicity of the suggestion in Shortcuts also work in rich text editors #7156 (comment), accept a patch to fix this (probably qualifies for the 5-good-beginner-bug label).

@singhvisha
Copy link

hi, I want to work on my very first bug...

@timvandermeij
Copy link
Contributor

timvandermeij commented Apr 17, 2019

Sure, please do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants