seed-rs/seed

unhandled error with browser autofill

Notmeor opened this issue · 4 comments

Notmeor commented

I built an admin spa with seed, and everything worked out great!

only that every time i log in, an error would pop up in browser console saying

panicked at 'called `Option::unwrap()` on a `None` value', /root/.cargo/registry/src/github.com-1ecc6299db9ec823/seed-0.8.0/src/browser/dom/event_handler.rs:48:59

Stack:

Error
    at imports.wbg.__wbg_new_59cb74e423758ede

it only happens when using password autofill, and except for the error msg, all functionalities seem intact.

MartinKavik commented

I built an admin spa with seed, and everything worked out great!

Glad to hear that!

Could you provide more details so I can fix it in a reasonable time, please? Ideally:

  • Your operating system
  • Browser
  • Do you use a plugin for autofil? (E.g. 1Password)
  • Could you write a minimal example with steps to reproduce the issue?
  • etc.

Thank you!

Notmeor commented

Sry for the late reponse.

  • os: MacOS Catalina, Version 10.15
  • browser: chrome/safari
  • plugin for autofill: no
  • how to reproduce: create a login form with user/password input, fill it a few times so that the browser would suggest an autofill

Here's the example


#![allow(clippy::wildcard_imports)]

use seed::{prelude::*, *};

const ENTER_KEY: u32 = 13;

#[derive(Default)]
struct Model {
    username: String,
    password: String,
}

fn init(_: Url, _: &mut impl Orders<Msg>) -> Model {
    Model::default()
}

enum Msg {
    UsernameChanged(String),
    PasswordChanged(String),
    RequestLogin,
}

fn update(msg: Msg, model: &mut Model, _: &mut impl Orders<Msg>) {
    match msg {
        Msg::UsernameChanged(username) => {
            model.username = username;
        },
        Msg::PasswordChanged(password) => {
            model.password = password;
        },
        Msg::RequestLogin => {
            log!("request login...");
        }
    }
}

#[allow(clippy::trivially_copy_pass_by_ref)]
// `view` describes what to display.
fn view(model: &Model) -> Node<Msg> {
    div![
        div![
             C!["field"],
             div![
                 C!["control"],
                 input![
                     C!["input is-medium"], 
                     attrs![
                         At::Type => "text",
                         At::Placeholder => "User",
                         At::Value => model.username,
                         At::AutoFocus => AtValue::None,
                     ],
                     input_ev(Ev::Input, Msg::UsernameChanged),
                 ]
             ]
         ],
         div![
             C!["field"],
             div![
                 C!["control"],
                 input![
                     C!["input is-medium"],
                     attrs![
                         At::Type => "password",
                         At::Placeholder => "Password",
                         At::Value => model.password,
                     ],
                     input_ev(Ev::Input, Msg::PasswordChanged),
                     keyboard_ev(Ev::KeyDown, |event| {
                         IF!(event.key_code() == ENTER_KEY => {
                             Msg::RequestLogin
                         })
                     })
                 ]
             ]
         ],
     ]
}

#[wasm_bindgen(start)]
pub fn start() {
    // Mount the `app` to the element with the `id` "app".
    App::start("app", init, update, view);
}
MartinKavik commented

Thanks! I can reproduce it with your example (on Windows 10, Chrome).
And I hate browser APIs and Javascript even more. And I need a bit help to decide how to fix it.
Let me explain:

The browser auto-fills the input and then fires the event with type set to keydown. However it's not a standard KeyboardEvent - the key_code field and probably multiple others are set to undefined. As the result, event casting in Seed fails and voilá here is your error.
When I change casting from event.dyn_ref to event.unchecked_ref, the check (by instanceof and probably other validations) is bypassed, but wasm_bindgen does another internal validation round and the app fails with a runtime error:

wasm-bindgen: imported JS function that was not marked as `catch` threw an error: expected a number argument
...
at web_sys::features::gen_KeyboardEvent::KeyboardEvent::key_code::h87a65aeba252d3a

-> it fails because it can't convert undefined to u32.

How we should treat that "malformed" KeyboardEvent? And should we try to change web_sys's methods like KeyboardEvent::KeyCode from key_code(&self) -> u32 to key_code(&self) -> Option<u32>? Is it a browser problem, WebIDL problem or wasm_bindgen problem or web_sys problem or Seed problem?

MartinKavik commented

I've done another research round.

Demonstration & explanation

Try https://jsfiddle.net/0mhwj6v5/1/ with opened console in dev tools on your browsers. You should see something like

image

or

image

The first image is a Firefox version. Autofill fires only input event (i.e. it doesn't fire keydown at all). And you can notice that the event's prototype is InputEventPrototype - so it should represent the correct object type/class InputEvent according to the specs.
When you select the entire input value and press the Delete key, it fires keydown and input events as expected with correct classes.

The second image is a Chrome version. Autofill fires both input and keydown and doesn't care about types at all - both events have just the general class Event, only their field type is set to "keydown" or "input".
When you select the entire input and press the Delete key, it fires both events with correct classes.

Bug reports / related issues

Expected behavior

I wasn't able to find official specs for autofill behavior.

I think the Firefox's behavior is the correct one - it doesn't make sense to fire keydown on autofill imho and its input event has the correct event class InputEvent.
Even if we decide to fire keydown event on autofill, it should have the class KeyboardEvent and the key field has to have the value "Unidentified" according to the specs - https://www.w3.org/TR/uievents-key/#key-attr-values. Other fields like keyCode should have a valid value, too.

General solution

I wasn't able to find a good solution that works on all browsers unfortunately. There are MANY tips, workaround and hacks, but most of them are either too old or too ugly. You can find workarounds like this one - https://github.com/paperjs/paper.js/pull/1358/files - in multiple projects.

Solution for Seed

The first idea:

  1. All specific event handlers (keyboard_ev, input_ev, etc.) would ignore incompatible incoming events - i.e. the handler wouldn't be invoked at all if it expects for instance KeyboardEvent but the event has class Event.
  2. The user would have to add a "raw" event handler ev(..) and handle/try to cast the event by himself.

Next steps

@Notmeor I would be glad if you try to create a PR, implementing the idea above or your idea (if you have any). Once we know it works on your Mac (+ Safari, Chrome, Firefox), I'll test it on Windows and merge it.

  • 👍1