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

Trying to enter text in comment box, typing `s` causes search to grab focus, eat letter. #1026

Closed
craigphicks opened this issue Mar 10, 2019 · 12 comments

Comments

@craigphicks
Copy link

commented Mar 10, 2019

Description

I added the "isso" self-administrated/serviced comment function to in place disqus and found the letter 's' (also 'S') can't be entered because that key causes the searchbox to grab the focus, eating the letter in the process.

I couldn't tell you if disqus avoids this because I haven't tried disqus.

I believe this is related to issue #400. One comment noted that s resulted in this behavior. That same commentator suggested removing attribute accesskey="s" from this element:

<input type="text" class="md-search__input" name="query" required="" placeholder="Search" accesskey="s" autocapitalize="off" autocorrect="off" autocomplete="off" spellcheck="false" data-md-component="query">

Indeed - that accesskey="s" attribute is now gone from that element, but the old behavior is currently present in this topic's scenario. My background is weak but I guess it might be getting trapped by some java code. However I cannot find it.

In issue #398 "Shortcut breaks forms" it the poster was advised to in a "form". I have tried that but it didn't seem to work - maybe cause it is wrapping a javascript? :

<form>
<div>
<script data-isso="https://pindertek.net/isso-2/"
        src="js-isso/embed.dev.js"></script>
<section id="isso-thread"></section>
</div>
</form>

I tried this both in the main text and overriding the {% block disqus %}, no difference.

As long as I don't type 's' or 'f' (any others?) I can successfully send a message.

Any help would be greatly appreciated.

This really is a great piece of software - kudo to squidfunk and to the whole stack.

Description of the bug

See "description"

What you expected to happen

Plain letters typed into a text don't cause another box to grab focus.

Actual behavior

See "description"

Steps to reproduce the bug

Go to (https://craigphicks.github.io/privca/) and scroll to bottom.
Try typing 'a' into comment box - it works.
Try typing 's' into comment box - the focus changes.

Package versions

  • Python: python --version

python 3.6.7

  • MkDocs: mkdocs --version

mkdocs, version 1.0.4 (Python 3.6)

  • Material: pip show mkdocs-material | grep -E ^Version
Name: mkdocs-material
Version: 4.0.2
Summary: A Material Design theme for MkDocs
Home-page: https://squidfunk.github.io/mkdocs-material/
Author: Martin Donath
Author-email: martin.donath@squidfunk.com
License: MIT
Location: /home/craig/github/privca/mkdocs.d/pyenv/lib/python3.6/site-packages
Requires: mkdocs, Pygments, pymdown-extensions

Project configuration

# Project information
site_name: 'Server/Client 2-way Authentication w/ Private CA'
site_description: 'Step by step guide to issuing private certs for Server and Client Side Authentication - specifically *lighttpd* and *Firefox*'
site_author: 'Craig Hicks'
site_url: 'https://craigphicks.github.io/privca'

# Repository
#repo_name: 'craigphicks/privca'
#repo_url: 'https://github.com/craigphicks/privca'

# Copyright
copyright: 'Copyright &copy; Craig Hicks 2019'

# Configuration
theme:
  name: 'material'
  language: 'en'
  palette:
    primary: 'indigo'
    accent: 'indigo'
  font:
    text: 'Roboto'
    code: 'Roboto Mono'

# Customization
extra:
  manifest: 'manifest.webmanifest'
  social:
  - type: 'github'
    link: 'https://github.com/craigphicks'
# #   - type: 'twitter'
# #     link: 'https://twitter.com/squidfunk'
# #   - type: 'linkedin'
# #     link: 'https://linkedin.com/in/squidfunk'

# Google Analytics
# google_analytics:
#   - 'UA-XXXXXXXX-X'
#   - 'auto'

# Extensions
markdown_extensions:
  - attr_list
  - admonition
  - codehilite:
      guess_lang: false
  - toc:
      permalink: true

#extra:
#  disqus: 'your-shortname'

theme:
  name: 'material'
  custom_dir: 'custom'

System information

  • OS: [The operating system you're running]

Linux ub18 4.18.0-15-generic #16~18.04.1-Ubuntu

  • Browser: [The browser used, if relevant]

Firefox 65.0

@squidfunk

This comment has been minimized.

Copy link
Owner

commented Mar 12, 2019

This is related to a div that is contenteditable, see #997. I will try and fix this when I find some time, but if somebody wants to go ahead, feel free to PR.

@squidfunk squidfunk added the bug label Mar 12, 2019

@squidfunk

This comment has been minimized.

Copy link
Owner

commented Mar 12, 2019

Potential fix in 1aa8aeb. Please check the latest master to verify if the issue is gone.

@craigphicks

This comment has been minimized.

Copy link
Author

commented Mar 13, 2019

Terrific - master verified (master branch forked and make'd) and the problem is gone on the public website. Thanks very much!

@squidfunk

This comment has been minimized.

Copy link
Owner

commented Mar 13, 2019

Great, I will issue a new release when #1023 is fixed.

@NicolasMassart

This comment has been minimized.

Copy link

commented Apr 4, 2019

I still have the issue when contenteditable is set to "inherit" like in Hotjar feedback form.
It looks like default value for textarea are contenteditable=inherit so it's not skipped and we can't input text in it as it loses focus. I tried to programatically change the contenteditable to true and then the skip works. Would skipping elements with tag name TEXTAREA a good workaround ?

@squidfunk

This comment has been minimized.

Copy link
Owner

commented Apr 4, 2019

inherit is not even a valid value according to the specs https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/contenteditable

I don’t really think going further into this direction will provide any value. Skipping textareas might work for your case but then other cases might emerge. I would really favor some research on what we need to do before adding more and more edge cases.

@NicolasMassart

This comment has been minimized.

Copy link

commented Apr 5, 2019

I agree, but I don't see what property we could target to prevent testing for this type of hedge case. Any idea ?

Does isContentEditable could be used instead ? https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/isContentEditable

And I had this page as reference https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/contentEditable stating that inherit is a possible value.
I would like to prevent having to fork for my own use as I prefer to have the real release. But hotjar is widely used, so maybe it could fix the issue for some other people.

@squidfunk

This comment has been minimized.

Copy link
Owner

commented Apr 5, 2019

Does isContentEditable could be used instead ? https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/isContentEditable

This looks like a good candidate solution that is generic. Browser compatibility looks promising, though no data on Opera and Internet Explorer. Could you adjust the source, test it (also in those browsers), and if it works make a PR? Happy to merge.

if (document.activeElement instanceof HTMLElement &&
document.activeElement.contentEditable === "true")
return

And I had this page as reference https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/contentEditable stating that inherit is a possible value.
I would like to prevent having to fork for my own use as I prefer to have the real release. But hotjar is widely used, so maybe it could fix the issue for some other people.

Oh, didn't know that. Also I understand that Hotjar is widely used. As stated above - I'm happy to collaborate on a PR.

@NicolasMassart

This comment has been minimized.

Copy link

commented Apr 5, 2019

I will test that and propose a PR if it works.

@NicolasMassart

This comment has been minimized.

Copy link

commented Apr 5, 2019

The issue I face is that TEXTAREA are not contentEditable, they are just natively capable to have text. So you can't base detection on these properties. It's the same thing for INPUT. The only thing is to test on tagName. What do you think ?

So my proposal would be to add :

      /* Skip natively text capable elements */
      if (document.activeElement.tagName==="TEXTAREA" || document.activeElement.tagName==="INPUT" )
        return

NicolasMassart added a commit to NicolasMassart/mkdocs-material that referenced this issue Apr 5, 2019

fixes squidfunk#1026 for TEXTAREA and INPUT elements
The bug affects HotJar feedback widget that loses focus in favour of the search
field each time you type s or f keys.

NicolasMassart added a commit to NicolasMassart/pantheon that referenced this issue Apr 5, 2019

hotjar tag
add hotjar tag and config value
override application JS to prevent search to conflict on focus.
See squidfunk/mkdocs-material#1026 (comment)
Override is temporary until a fix is done in mkdocs-material
@NicolasMassart

This comment has been minimized.

Copy link

commented Apr 5, 2019

I just pushed PR #1060 with my way to solve the issue, let me know if you like it. No problem if you don't, I will just use it for my site, and I would be very happy if you find a better way to fix this.

NicolasMassart added a commit to PegaSysEng/pantheon that referenced this issue Apr 5, 2019

hotjar tag (#1225)
add hotjar tag and config value
override application JS to prevent search to conflict on focus.
See squidfunk/mkdocs-material#1026 (comment)
Override is temporary until a fix is done in mkdocs-material
@CesMak

This comment has been minimized.

Copy link

commented Apr 20, 2019

Well for a quick fix I tried to change
/home/name/anaconda3/lib/python3.6/site-packages/material/assets/javascripts/application.8eb9be28.js:1

the line: 

...
document.activeElement&&!document.activeElement.form&&(70!==e.keyCode&&83!==e.keyCode||(n.focus(),e.preventDefault()))}).listen(),new f.default.Event.Listener(window,"keypress",function(...

to any other keycode: 70 --> 10 etc.
however this had no impact at all... any ideas why?

This is my material pip setup:

Version: 2.7.0
Summary: A Material Design theme for MkDocs
Home-page: https://squidfunk.github.io/mkdocs-material/
Author: Martin Donath
Author-email: martin.donath@squidfunk.com
License: MIT
Requires: Pygments, pymdown-extensions, tornado, mkdocs

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