Support mozbrowsericonchange event (Browser API)

[paulrouget]

No description provided.

[paulrouget]

I'm giving it a try: paulrouget@97cee8b

At the moment, we are limited by the event.detail property. It only supports strings (see https://github.com/servo/servo/blob/master/components/script/dom/htmliframeelement.rs#L133).

@glennw what would you recommend here? MozBrowserEvent::detail() returns a string. Maybe it should return a json-like data structure (hashmap?) that would be converted to jsval in HTMLIframeElement::dispatch_mozbrowser_event() ? But I'm not sure a hashmap is good enough. Do we have a generic JSON-like structure in Rust/Servo? Or should we translate to a jsval directly in MozBrowserEvent::detail()?

[jdm]

We have two JSON representations (first, second), but I propose making detail() take a MutHandleValue argument and directly converting the values rather than having a rust-typed common return.

[paulrouget]

@jdm I'm trying to use to_jsval in msg/constellation_msg.rs, so I guess I need use script::dom::bindings::conversions::{ToJSValConvertible}; which requires the script crate. Adding the script dependency in msg end up with a cyclic package dependency:

cyclic package dependency: package `devtools_traits v0.0.1 (file:///Users/paul/git/servo/components/servo)` depends on itself

What's the right approach in this case?

[eefriedman]

I'm trying to use to_jsval in msg/constellation_msg.rs

Don't do that. You shouldn't be handling a JSContext outside of the script crate/thread.

[jdm]

The right approach is to make 'detail' a method of a trait that's defined in the script crate.

[paulrouget]

moving detail into the script crate works, but I'm having difficulties building the JS object.

• Should I build the object manually with RootedObject::new and JS_DefineProperty (or define_properties)?
• Should I build the object via to_jsval? This might require adding a new conversion method conversions.rs
• Would creating a BrowserIconChangeEventDetail interface in BrowserElement.webidl help in any way?

[jdm]

Creating a BrowserIconChangeEventDetail dictionary is probably the way to go, rather than an interface. That should allow us to fill out the properties of the corresponding Rust object and then calling to_jsval.

[paulrouget]

feedback? paulrouget@2f5fa1b

Does this look like the right approach?

Still need to do some clean-up, address the fixme, and write some tests

[jdm]

It does!