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

Some sites may remove listeners on blur #213

Open
luisherranz opened this issue Aug 15, 2021 · 14 comments
Open

Some sites may remove listeners on blur #213

luisherranz opened this issue Aug 15, 2021 · 14 comments

Comments

@luisherranz
Copy link
Contributor

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.

ScreenFlow.mp4

I'll open a PR to fix it.

@fregante
Copy link
Owner

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

// These are in the right order
this.field.dispatchEvent(new KeyboardEvent('keydown'), eventOptions);
this.field.dispatchEvent(new KeyboardEvent('keypress'), eventOptions);
this.field.dispatchEvent(new CompositionEvent('textInput'), eventOptions);
this.field.dispatchEvent(new CustomEvent('input', { // InputEvent doesn't support custom data
...eventOptions,
detail: {
ghostTextSyntheticEvent: true
}
}));
this.field.dispatchEvent(new KeyboardEvent('keyup'), eventOptions);

@luisherranz
Copy link
Contributor Author

luisherranz commented Aug 15, 2021

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

@fregante
Copy link
Owner

Screen Shot 5

It looks like GitHub is listening to the input event

@fregante
Copy link
Owner

It works for me (Safari)

gif

@fregante
Copy link
Owner

fregante commented Aug 15, 2021

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
Copy link
Contributor Author

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

@luisherranz
Copy link
Contributor Author

luisherranz commented Aug 15, 2021

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
Copy link
Contributor Author

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
Copy link
Owner

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

@luisherranz
Copy link
Contributor Author

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: #53 (comment)

@fregante
Copy link
Owner

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

@luisherranz
Copy link
Contributor Author

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
Copy link
Owner

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.

@luisherranz
Copy link
Contributor Author

Ok, I guess it's worth a try 🙂

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

@fregante fregante changed the title Sites that depend on the "change" event don't update Some sites may remove listeners on blur Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants