search

sixcious / url-incrementer

Increment a URL or go to the next page. Supports auto, multi, and advanced incrementing functions.
Other
42 stars 3 forks source link

Shortcuts interfere with text editing #6

Closed alexolog closed 3 years ago

alexolog commented 4 years ago
  1. Navigate to https://forums.getpaint.net/topic/4510-rants-enter-at-own-risk
  2. Go to the bottom of the page, click in "reply to this topic"
  3. In the edit box that opens, type a word, then press SHIFT-CONTROL-[left arrow]

Expected outcome: The word gets selected Actual outcome: The word gets selected, then the browser jumps to the previous page in the thread.

This should not happen when the caret is inside anything that behaves like an editor field.

sixcious commented 4 years ago

Hi alex,

Thanks for opening this issue!

I apologize if URLI caused you to lose any work while you were typing in that edit box.

URLI has two types of shortcuts: Browser Shortcuts and Internal Shortcuts. Unfortunately, we have no way of controlling how Browser Shortcuts work; that's up to Chrome/FireFox/Edge to implement. As far as URLI's own Internal Shortcuts, this has already been coded in the current build of URLI here on GitHub and in the FireFox version, where it won't fire the shortcut if it detects you're typing in any input, textarea, or element that isContentEditable:

https://github.com/roysix/url-incrementer/blob/d44ca56a1e2c757cfedeaac9b6066affbd9677af/src/chrome/js/shortcuts.js#L120-L135

Unfortunately it's not yet in the Chrome Web Store (CWS) version yet, which is a little behind on this point. Hopefully, I can get it out there later this year, but it's really difficult to put an update on the Web Store right now. If you're using the CWS version, I would recommend using the Internal Shortcuts and add an extra modifier key (e.g. Ctrl+Shift+Alt) to avoid this from happening.

Hope that helps

alexolog commented 4 years ago

I am using Firefox and the element in my example is not detected.

In particular, looking at the source shows:

<div class="cke_wysiwyg_div cke_reset cke_enable_context_menu cke_editable cke_editable_themed cke_contents_ltr" hidefocus="true" tabindex="1" spellcheck="true" role="textbox" aria-multiline="true" aria-label="Enter your text; hold ctrl and right click for more options" title="Enter your text; hold ctrl and right click for more options" data-gramm="false" style="max-height: 766px;" contenteditable="true"><p><br></p></div>

which is not detected by your code.

sixcious commented 4 years ago

OK, thanks for letting me know what version you are using, as that kind of makes a big difference!

It looks like I misremembered about the FireFox version. In the FF version, we do check for input and textarea, but not for isContentEditable. I added the isContentEditable check afterwards since I wasn't aware of it at the time the FF version was released two years ago.

This is what the FF version code only checks for: if (nodeName === "INPUT" || nodeName === "TEXTAREA") { return; }

So unfortunately, in your situation, since the website only uses a DIV and contenteditable the only option for now is to change the shortcut (e.g. add an extra modifier key like Alt). Or you could use the local version here on GitHub, which already has this issue addressed. I'll probably use a different default shortcut for Next/Prev as well just to avoid this issue completely.

alexolog commented 4 years ago

So why not add the check for isContentEditable to the FF version regardless of the node name?

alexolog commented 4 years ago

https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/isContentEditable has a nice example where a <p> element is editable

sixcious commented 4 years ago

Sorry for the delay and thanks for the link.

Yes, that's the plan! I plan on updating the Firefox version soon with the current code base here on GitHub, which has the isContentEditable check in place, among some new features. Apologies, as all my free time has been held up with a new extension that I just released, so I haven't had the time to work on URLI much lately.

Thank you again for bringing up this issue, and reminding me about the FF version. And sorry once again for any time you lost due to this.

alexolog commented 4 years ago

No problem. Thanks for maintaining it! Please comment here when the new version is available.

alexolog commented 3 years ago

Hi, has there been any progress? Thank you.

sixcious commented 3 years ago

Hey again!

Yes, this is still on track. Sorry, for the delay on this. All my time has been taken up by Infy Scroll, which I'm close to wrapping up on for the year. I was aiming to get URLI's 6.0 update out in early December, which will have this feature, among a bunch of other things.

Apologies, I guess I tend not to do a lot of small bugfix releases because they take up time to test and package each one. Is it possible for you to wait until December's 6.0 release? I suppose I could try and mangle up a "0.1" update sooner, as this is a simple one-line change that wouldn't require a lot of testing... if 6.0 isn't out soon enough, I'll do that.

Thanks for your patience

alexolog commented 3 years ago

Infy scroll looks very interesting. Will it be possible to bounce some ideas off of you? If so, where would be a good place to do it?

sixcious commented 3 years ago

Sure thing! Feel free to email or post another issue. If it's something that I can implement, you'll be credited.

Thanks again.

sixcious commented 3 years ago

Hi Alex. This has now been fixed in URL Incrementer Firefox Version 0.1, released on Feb 1, 2021. Thank you for your patience and for opening this issue.

And thanks for using URLI!