overlays: Fix stream edit click-through bug.

[YashRE42]

Fixes ##12369.

Testing Plan:

GIFs or Screenshots:

side note: This was a terrible experience, it ate a lot of time. (but that is my fault for trying to find a good commit for a bisect before checking simple stuff first). That said, do our dev docs say anything about e.stopPropagation(); and if not should we consider something?

[timabbott]

Nice, merged, thanks @YashRE42!

This is a common category of click handler bug. As a general style rule, we try to use stopPropagation and preventDefault in essentially all of our focused click handlers (where you're clicking a specific button or area), but inevitable we end up merging some that don't have it, and the bugs it produces are almost always super annoying to track down just like this one.

It is probably worth doing an audit for similar issues in our click handlers, though we do need to be really careful to not just blindly find/replace, because some click handlers have cases where they want the default browser behavior to run. Possibly we should add comments for all of those with that property.

Interested in working on that?

[YashRE42]

Would you have a strategy in mind for trying to get all the handlers?

[timabbott]

There's this:

(zulip-py3-venv) tabbott@coset:~/zulip$ git grep '[.]on[(]' static/js/ | wc
    428    2455   40753

to find which files to look through. It's a lot of call points, but I think it should be a pretty fast scan, since most of them will just have a .stopPropagation clearly there.

[YashRE42]

I'm wondering if we can write some regex to at least eliminate all the handlers which already have .stopPropagation (which would also catch those that already have comments) from that count. I'll create an issue and track the progress there.

[YashRE42]

as further thought, if it's possible I think it'd be brilliant to add a rule in our linter to auto-catch this kind of thing. Essentially, for every .on handler, either the next brace pair should contain .stopPropagation or should have a comment that explicitly states why it's not there (and use the exact string .stopPropagation and hence also be excluded). I'll look into it, because we could put the linter in place first and then just look at all the handlers it points out.