Make right-click work on all browsers #342
Conversation
src/utils/Generic.js
Outdated
| * @module xterm/utils/Generic | ||
| */ | ||
|
|
||
| export let contains = function(el, arr) { |
There was a problem hiding this comment.
arr, el would be more intuitive ordering imo "does arr contain el"
src/utils/Generic.js
Outdated
| */ | ||
|
|
||
| export let contains = function(el, arr) { | ||
| for (var i = 0; i < arr.length; i += 1) { |
src/xterm.js
Outdated
| }); | ||
| } | ||
|
|
||
| if (isFirefox || isMSIE) { |
There was a problem hiding this comment.
Is it a concern you're forking the logic here? If both work equally well with mousedown maybe that would be better so all browsers use the same code path?
There was a problem hiding this comment.
Yep, it doesn't work on Chrome with mousedown. The "Paste" option is disabled.
| term.textarea.style.height = '10px'; | ||
| term.textarea.style.left = x + 'px'; | ||
| term.textarea.style.top = y + 'px'; | ||
| term.textarea.style.width = '20px'; |
There was a problem hiding this comment.
Curious why this is bigger all of a sudden?
There was a problem hiding this comment.
Just gave it a little bit of padding, because I think it could run into corner cases.
parisk
changed the title
|
@Tyriar do you have any clue why this particular test might be failing? |
|
@parisk probably related to |
|
Thanks @Tyriar, the error message wasn't that helpful 😅 . I was setting |
|
@Tyriar tests are passing, feel free to take another look, when it's 🆗 for you. |
Made right-click work on all browsers.
The issue with the previous implementation was that it was bound to the
contextmenuDOM event.While this works for Chrome and Safari (and Edge?), Firefox and Internet Explorer fire the
contextmenuevent after the menu appears (thus highjacking the textarea position is pointless). So, I moved on and detected right-click using themousedownDOM event on Firefox and MSIE.In addition, I moved all browser detecting utilities into a new
utils/Browser.jsmodule.I would like to add some tests for this module but I couldn't find a way to make it work with Mocha, since
navigatoris absent.Closes #310