fregante / GhostText

👻 Use your text editor to write in your browser. Everything you type in the editor will be instantly updated in the browser (and vice versa).
https://GhostText.fregante.com
MIT License
3.31k stars 117 forks source link

Some sites may remove listeners on `blur` #213

Open luisherranz opened 3 years ago

luisherranz commented 3 years ago

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.

fregante commented 3 years ago

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

luisherranz commented 3 years ago

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

fregante commented 3 years ago
Screen Shot 5

It looks like GitHub is listening to the input event

fregante commented 3 years ago

It works for me (Safari)

gif

fregante commented 3 years ago

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 🤔

luisherranz commented 3 years ago

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

luisherranz commented 3 years ago

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 commented 3 years ago

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? 🙂

fregante commented 3 years ago

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

luisherranz commented 3 years ago

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

fregante commented 3 years ago

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

luisherranz commented 3 years ago

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.

fregante commented 3 years ago

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:

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.

luisherranz commented 3 years ago

Ok, I guess it's worth a try 🙂

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