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

Implement EventListenerOptions for EventTarget #9785

Closed
KiChjang opened this issue Feb 27, 2016 · 11 comments
Closed

Implement EventListenerOptions for EventTarget #9785

KiChjang opened this issue Feb 27, 2016 · 11 comments

Comments

@KiChjang
Copy link
Member

@KiChjang KiChjang commented Feb 27, 2016

The WebIDL file that we have currently reflects an older version of the DOM spec. We should update this to use EventListenerOptions accordingly.

Code: components/script/dom/webidls/EventTarget.webidl, components/script/dom/eventtarget.rs
Spec: https://dom.spec.whatwg.org/#interface-eventtarget

@Ms2ger
Copy link
Contributor

@Ms2ger Ms2ger commented Feb 27, 2016

I'm not sure there's consensus on this feature yet, so I'd hold off on implementing for now.

@nox
Copy link
Member

@nox nox commented Jun 4, 2016

Ah, damn... Cc @GuillaumeGomez

@jdm
Copy link
Member

@jdm jdm commented Jun 4, 2016

The Gecko/chrome meeting from a month or two ago seemed to believe this was desirable.

@GuillaumeGomez
Copy link
Contributor

@GuillaumeGomez GuillaumeGomez commented Jun 4, 2016

I take it?

@nox
Copy link
Member

@nox nox commented Jun 4, 2016

Sure.

@nox nox added the C-assigned label Jun 4, 2016
@nox
Copy link
Member

@nox nox commented Jun 4, 2016

Blocked by #11612.

@GuillaumeGomez
Copy link
Contributor

@GuillaumeGomez GuillaumeGomez commented Jun 4, 2016

The patch awaits for the previous issue to be solved.

@Ms2ger
Copy link
Contributor

@Ms2ger Ms2ger commented Jan 20, 2017

Partial fix:

diff --git a/components/script/dom/eventtarget.rs b/components/script/dom/eventtarget.rs
index 1023b97..13e1343 100644
--- a/components/script/dom/eventtarget.rs
+++ b/components/script/dom/eventtarget.rs
@@ -14,6 +14,7 @@ use dom::bindings::codegen::Bindings::EventHandlerBinding::OnErrorEventHandlerNo
 use dom::bindings::codegen::Bindings::EventListenerBinding::EventListener;
 use dom::bindings::codegen::Bindings::EventTargetBinding::EventTargetMethods;
 use dom::bindings::codegen::Bindings::WindowBinding::WindowMethods;
+use dom::bindings::codegen::UnionTypes::EventListenerOptionsOrBoolean;
 use dom::bindings::codegen::UnionTypes::EventOrString;
 use dom::bindings::error::{Error, Fallible, report_pending_exception};
 use dom::bindings::inheritance::Castable;
@@ -547,11 +548,15 @@ impl EventTargetMethods for EventTarget {
     fn AddEventListener(&self,
                         ty: DOMString,
                         listener: Option<Rc<EventListener>>,
-                        capture: bool) {
+                        capture: EventListenerOptionsOrBoolean) {
         let listener = match listener {
             Some(l) => l,
             None => return,
         };
+        let capture = match capture {
+            EventListenerOptionsOrBoolean::EventListenerOptions(o) => o.capture,
+            EventListenerOptionsOrBoolean::Boolean(b) => b,
+        };
         let mut handlers = self.handlers.borrow_mut();
         let entry = match handlers.entry(Atom::from(ty)) {
             Occupied(entry) => entry.into_mut(),
diff --git a/components/script/dom/mediaquerylist.rs b/components/script/dom/mediaquerylist.rs
index c6e569b..bd31bf3 100644
--- a/components/script/dom/mediaquerylist.rs
+++ b/components/script/dom/mediaquerylist.rs
@@ -7,6 +7,7 @@ use dom::bindings::codegen::Bindings::EventHandlerBinding::EventHandlerNonNull;
 use dom::bindings::codegen::Bindings::EventListenerBinding::EventListener;
 use dom::bindings::codegen::Bindings::EventTargetBinding::EventTargetMethods;
 use dom::bindings::codegen::Bindings::MediaQueryListBinding::{self, MediaQueryListMethods};
+use dom::bindings::codegen::UnionTypes::EventListenerOptionsOrBoolean;
 use dom::bindings::inheritance::Castable;
 use dom::bindings::js::{JS, Root};
 use dom::bindings::reflector::DomObject;
@@ -107,7 +108,8 @@ impl MediaQueryListMethods for MediaQueryList {
     // https://drafts.csswg.org/cssom-view/#dom-mediaquerylist-addlistener
     fn AddListener(&self, listener: Option<Rc<EventListener>>) {
         self.upcast::<EventTarget>().AddEventListener(DOMString::from_string("change".to_owned()),
-                                                      listener, false);
+                                                      listener,
+                                                      EventListenerOptionsOrBoolean::Boolean(false));
     }
 
     // https://drafts.csswg.org/cssom-view/#dom-mediaquerylist-removelistener
diff --git a/components/script/dom/webidls/EventTarget.webidl b/components/script/dom/webidls/EventTarget.webidl
index ee6e5d7..9c9089b 100644
--- a/components/script/dom/webidls/EventTarget.webidl
+++ b/components/script/dom/webidls/EventTarget.webidl
@@ -5,11 +5,16 @@
  * https://dom.spec.whatwg.org/#interface-eventtarget
  */
 
+dictionary EventListenerOptions {
+  boolean capture = false;
+};
+
+
 [Abstract, Exposed=(Window,Worker)]
 interface EventTarget {
   void addEventListener(DOMString type,
                         EventListener? listener,
-                        optional boolean capture = false);
+                        optional (EventListenerOptions or boolean) capture);
   void removeEventListener(DOMString type,
                            EventListener? listener,
                            optional boolean capture = false);
Ms2ger added a commit that referenced this issue Jan 20, 2017
@KiChjang
Copy link
Member Author

@KiChjang KiChjang commented Mar 2, 2017

Not sure if @GuillaumeGomez is still working on this.

@GuillaumeGomez
Copy link
Contributor

@GuillaumeGomez GuillaumeGomez commented Mar 2, 2017

I am, more or less. Quite busy unfortunately.

@nox
Copy link
Member

@nox nox commented Sep 30, 2017

Superseded by #13242 and #13243.

@nox nox closed this Sep 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

5 participants
You can’t perform that action at this time.