Fixed keydown event handling problem with shadow DOM. #12405
Conversation
When does this actually happen, since as-is there's not really enough information provided to make it possible to understand in exactly what situations these changes are needed. Please provide additional information, and preferably a runnable example such that it's possible to actually test/review these changes! Furthermore, given that the default viewer was originally written for usage in Firefox, it's somewhat difficult to even understand why these any changes are necessary here. For example, why can't these changes simply be included in a custom implementation of the viewer; please note http://mozilla.github.io/pdf.js/getting_started/#introduction (emphasis mine):
|
|
Sorry for the information missing. I am developing a translator extension for multiple browsers and I used pdf.js as a built-in pdf viewer in my extension. When I am trying injecting a shadow DOM with an input box into the pdf viewer, I found that I could not input space in the input box. When I clicked the space key, the keydown event was catched by pdf.js and resulted in page scrolling instead of inputting a space in the input box. After digging into the code I found that although pdf.js had considered the input case, the original solution could not handle DOMs with shadow DOM element. I also found that the problem with text editors has been discussed in #7156 and #11253 provided the incomplete solution. So I thought that maybe you need a patch for that. |
web/ui_utils.js
Outdated
| /** | ||
| * Get the active or focused element in current DOM. | ||
| * | ||
| * Recursively search for the truly active or focused element incase there are |
web/ui_utils.js
Outdated
| * Recursively search for the truly active or focused element incase there are | ||
| * shadow DOMs. | ||
| * | ||
| * @author Chen Jinlong <chenjinlong2016@outlook.com> |
There was a problem hiding this comment.
We never use the @author tag in the codebase, so this should be removed.
web/ui_utils.js
Outdated
| * | ||
| * @author Chen Jinlong <chenjinlong2016@outlook.com> | ||
| * | ||
| * @param {DocumentOrShadowRoot} root the root element. |
There was a problem hiding this comment.
@param {DocumentOrShadowRoot} [root] - The root element.
root should be in brackets to indicate that it's an optional argument.
We can also leave the entire root argument out since no code currently calls this function with an argument?
web/ui_utils.js
Outdated
| * @author Chen Jinlong <chenjinlong2016@outlook.com> | ||
| * | ||
| * @param {DocumentOrShadowRoot} root the root element. | ||
| * |
There was a problem hiding this comment.
This newline can be removed.
|
Sorry for the late replying. I'll fix the comment problems and make a package for you to verify the patch. |
Editable elements in shadow DOMs can not be detected in old version.
nickyc975
force-pushed
the
fixed-active-element-in-shadow-dom
branch
from
07a34f0
to
252e258
Compare
|
I made 2 packages for you to verify the patch. One is built from the fixed branch and the other is built from the original branch. Download the packages and extract them to somewhere in you computer respectively. Follow the following steps to run the packages. Run original:
Run fixed: Steps 1-4 is the same (replace Here are the packages: fixed: fixed.zip original: original.zip |
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/202ae21a71a27c7/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/202ae21a71a27c7/output.txt Total script time: 3.58 mins Published |
|
Seems to work just fine. Thanks! |
Editable elements in shadow DOMs can not be detected in old version.