GhostText icon indicating copy to clipboard operation
GhostText copied to clipboard

Some sites may remove listeners on `blur`

Open luisherranz opened this issue 2 years ago • 14 comments

Setup

Browser: Chrome Editor: VS Code

Description

Sites that listen to the change event of the textarea don't update.

One example is GitHub's "Comment" button of issues and PRs, which doesn't update when the text is written via GhostText.

https://user-images.githubusercontent.com/3305402/129475148-31d3bf8b-a082-4bfb-a1e4-0210f1a3ff27.mp4

I'll open a PR to fix it.

luisherranz avatar Aug 15 '21 17:08 luisherranz

Thanks for the video! Indeed change is not among the faked events. PR welcome to add it in the right order

https://github.com/fregante/GhostText/blob/ad87dffac1e1355e44e7c240a5e571575280b7af/source/ghost-text.js#L164-L174

fregante avatar Aug 15 '21 17:08 fregante

I guess it makes sense for it to be the last one dispatched. What do you think?

luisherranz avatar Aug 15 '21 17:08 luisherranz

Screen Shot 5

It looks like GitHub is listening to the input event

fregante avatar Aug 15 '21 17:08 fregante

It works for me (Safari)

gif

fregante avatar Aug 15 '21 17:08 fregante

Heh, the code I posted earlier seems to be the problem. It's not GhostText but GitHub itself is removing the event handler on blur. When GhostText is focused, the field loses the focus and GitHub detaches the "Comment" event handler.

You can see the "Close with comment" keeps working even in Chrome.

To fix this I should trigger blur rather than change, but only where necessary. I'm not sure this is worth the potential issues that a fake blur event might cause 🤔

fregante avatar Aug 15 '21 17:08 fregante

Maybe Safari and Chrome are doing different things? For me adding the change event fixed the problem in Chrome.

luisherranz avatar Aug 15 '21 17:08 luisherranz

Ok, I've been debugging GitHub's code and you're absolutely right, I ended up in the same place:

// Observe required fields and validate form when their validation state
// changes.
onFocus(supportedFields, field => {
  let previousValid = (field as Field).checkValidity()
  function inputHandler() {
    const currentValid = (field as Field).checkValidity()
    if (currentValid !== previousValid && (field as Field).form) {
      validate((field as Field).form!)
    }
    previousValid = currentValid
  }

  field.addEventListener('input', inputHandler)
  field.addEventListener('blur', function blurHandler() {
    field.removeEventListener('input', inputHandler)
    field.removeEventListener('blur', blurHandler)
  })
})

I'll try to see if I can see why adding the change event on each keystroke fixes this problem.

luisherranz avatar Aug 15 '21 18:08 luisherranz

Ok, saw it. It's because there is also another listener in the form for change events that also enables/disables the button:

// Install validation handlers on a form.
function installHandlers(form: HTMLFormElement) {
  if (installedForms.get(form)) return
  form.addEventListener('change', () => validate(form))
  installedForms.set(form, true)
}

// Validate a form or input.
export function validate(form: HTMLInputElement | HTMLFormElement) {
  const validity = form.checkValidity()
  for (const button of form.querySelectorAll<HTMLButtonElement>('button[data-disable-invalid]')) {
    button.disabled = !validity
  }
}

So it is listening to the input event when the element has the focus, and the change event in the form element.

I agree with you that sending a change event on each keystroke is not a good solution if that's not what the browser usually does.

Any other ideas? 🙂

luisherranz avatar Aug 15 '21 18:08 luisherranz

Other than re-focusing the field I don’t think that there's anything else. Maybe preventing the first blur event via stopImmediatePropagation?

fregante avatar Aug 15 '21 18:08 fregante

In my opinion both sound a bit hacky. Sorry, I thought this had a simple fix.

The only solution that seems rather "safe" to me is to apply this fix only for github.com, in case you ever accept some type of "per site transformation" functionality as we are talking about here: https://github.com/fregante/GhostText/issues/53#issuecomment-899087686

luisherranz avatar Aug 15 '21 20:08 luisherranz

The blur event could be captured by GhostText and immediately stopped before switching to the editor

fregante avatar Sep 18 '21 13:09 fregante

Hmm... I guess the problem is when to retrigger it again, don't you think? If we just swallow the blur event then we may break sites that depend on it for some behaviour.

luisherranz avatar Sep 18 '21 13:09 luisherranz

Once the user returns to the browser, the field will be focused again and the regular flow will resume.

Either blocking the blur or triggering a fake focus, the application will be in the same state:

  • The field is "focused" even if it's not, so once the user returns, it will trigger a real focus event

I think it's easier/safer to avoid the first blur altogether.

In this specific instance the issue is that events are disconnected on blur, so this seems to be the exact fix for the issue… however I doubt there's any perfect fix.

fregante avatar Sep 18 '21 14:09 fregante

Ok, I guess it's worth a try 🙂

Do you know of any other sites that depend on this apart from GitHub?

luisherranz avatar Sep 19 '21 09:09 luisherranz