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

Event propagation cancellation check happens too early? #2185

Closed
1 of 3 tasks
Tracked by #2052
rjmac opened this issue Nov 23, 2021 · 1 comment · Fixed by #2191
Closed
1 of 3 tasks
Tracked by #2052

Event propagation cancellation check happens too early? #2185

rjmac opened this issue Nov 23, 2021 · 1 comment · Fixed by #2191
Labels
A-yew Area: The main yew crate bug

Comments

@rjmac
Copy link

rjmac commented Nov 23, 2021

Problem

I think the check for event propagation cancellation happens too early. In particular, I think that

        if unsafe { BUBBLE_EVENTS } && !event.cancel_bubble() {
            let mut el = target;
            loop {
                el = match el.parent_element() {

ought to be

        if unsafe { BUBBLE_EVENTS } {
            let mut el = target;
            while !event.cancel_bubble() {
                el = match el.parent_element() {

Steps To Reproduce

Using this simple program:

use yew::prelude::*;
use gloo_console::log;

struct Demo;

pub enum Msg {
  OuterClicked(MouseEvent),
  InnerClicked(MouseEvent)
}

impl Component for Demo {
    type Properties = ();
    type Message = Msg;

    fn create(_ctx: &Context<Self>) -> Self {
        Self
    }

    fn update(&mut self, _ctx: &Context<Self>, msg: Self::Message) -> bool {
        match msg {
            Msg::OuterClicked(_) => {
                log!("Outer clicked");
                false
            }
            Msg::InnerClicked(ev) => {
                ev.stop_propagation();
                log!("Inner clicked");
                false
            }
        }
    }

    fn view(&self, ctx: &Context<Self>) -> Html {
        html!{
            <div onclick={ctx.link().callback(Msg::OuterClicked)}>
                <div onclick={ctx.link().callback(Msg::InnerClicked)}>
                    {"Some text"}
                    <div>{"And then a nested div"}</div>
                 </div>
            </div>
        }
    }
}

fn main() {
    ::yew::start_app::<Demo>();
}
  1. Click on "Some text". Observe it logs "Inner clicked".
  2. Click on "And then a nested div". Observe it logs both "Inner clicked" and "Outer clicked".
  3. Change yew's code as described at the top.
  4. Now clicking "And then a nested div" logs only "Inner clicked".

Expected behavior
Propagation should be cancelled even if the listener is not attached to the innermost element of the DOM element to which the event was first delivered.

Environment:

  • Yew version: master
  • Rust version: 1.56.1
  • Target, if relevant: wasm32-unknown-emscripten (probably not relevant)
  • Build tool, if relevant: trunk (proably not relevant)
  • OS, if relevant: Linux (probably not relevant)
  • Browser and version, if relevant: Firefox 94.0.2

Questionnaire

  • I'm interested in fixing this myself but don't know where to start
  • I would like to fix and I have a solution
  • I don't have time to fix this right now, but maybe later

If you the maintainers like the fix described at the top I can turn this into a pull request. Or you can do it if you'd rather avoid a roundtrip. Alternately, if this isn't the right change and you can describe how it's wrong, I can create a PR with the right change instead.

@rjmac rjmac added the bug label Nov 23, 2021
@ranile ranile added the A-yew Area: The main yew crate label Nov 23, 2021
@mc1098 mc1098 closed this as completed Nov 23, 2021
@mc1098 mc1098 reopened this Nov 23, 2021
@mc1098
Copy link
Contributor

mc1098 commented Nov 23, 2021

If you the maintainers like the fix described at the top I can turn this into a pull request.

@rjmac Good catch and yes please! :D

Sorry for the accidental close :(

rjmac pushed a commit to rjmac/yew that referenced this issue Nov 23, 2021
Rather than once after the innermost node is given the chance to
handle it.

Fixes yewstack#2185
voidpumpkin pushed a commit that referenced this issue Nov 23, 2021
Rather than once after the innermost node is given the chance to
handle it.

Fixes #2185

Co-authored-by: Robert Macomber <robertm@mox>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-yew Area: The main yew crate bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants