Auto merge of #15142 - nox:beforeunload, r=<try>

bors-servo · web-flow · commit 01dbaa505c96 · 2017-01-25T04:05:39.000-08:00
Properly cancel event when beforeunload handler returns null  This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15142)
diff --git a/components/script/dom/beforeunloadevent.rs b/components/script/dom/beforeunloadevent.rs
@@ -15,6 +15,7 @@ use dom::bindings::str::DOMString;
 use dom::event::{Event, EventBubbles, EventCancelable};
 use dom::window::Window;
 use servo_atoms::Atom;
+use std::cell::Ref;
 
 // https://html.spec.whatwg.org/multipage/#beforeunloadevent
 #[dom_struct]
@@ -49,6 +50,10 @@ impl BeforeUnloadEvent {
         }
         ev
     }
+
+    pub fn return_value(&self) -> Ref<DOMString> {
+        self.return_value.borrow()
+    }
 }
 
 impl BeforeUnloadEventMethods for BeforeUnloadEvent {
diff --git a/components/script/dom/eventtarget.rs b/components/script/dom/eventtarget.rs
@@ -178,22 +178,15 @@ impl CompiledEventListener {
                     }
 
                     CommonEventHandler::BeforeUnloadEventHandler(ref handler) => {
-                        if let Some(event) = event.downcast::<BeforeUnloadEvent>() {
-                            let rv = event.ReturnValue();
-
-                            if let Ok(value) = handler.Call_(object,
-                                                             event.upcast::<Event>(),
-                                                             exception_handle) {
-                                match value {
-                                    Some(value) => {
-                                        if rv.is_empty() {
-                                            event.SetReturnValue(value);
-                                        }
-                                    }
-                                    None => {
-                                        event.upcast::<Event>().PreventDefault();
+                        if let Ok(value) = handler.Call_(object, event, exception_handle) {
+                            if let Some(return_value) = value {
+                                if let Some(before_unload_event) = event.downcast::<BeforeUnloadEvent>() {
+                                    if before_unload_event.return_value().is_empty() {
+                                        before_unload_event.SetReturnValue(return_value);
                                     }
                                 }
+                            } else {
+                                event.PreventDefault();
                             }
                         }
                     }
diff --git a/tests/wpt/metadata/html/webappapis/scripting/events/event-handler-processing-algorithm.html.ini b/tests/wpt/metadata/html/webappapis/scripting/events/event-handler-processing-algorithm.html.ini

Original file line number	Diff line number	Diff line change
`@@ -15,6 +15,7 @@ use dom::bindings::str::DOMString;`
`15`	`15`	`use dom::event::{Event, EventBubbles, EventCancelable};`
`16`	`16`	`use dom::window::Window;`
`17`	`17`	`use servo_atoms::Atom;`
	`18`	`+use std::cell::Ref;`
`18`	`19`
`19`	`20`	`// https://html.spec.whatwg.org/multipage/#beforeunloadevent`
`20`	`21`	`#[dom_struct]`
`@@ -49,6 +50,10 @@ impl BeforeUnloadEvent {`
`49`	`50`	`}`
`50`	`51`	`ev`
`51`	`52`	`}`
	`53`	`+`
	`54`	`+ pub fn return_value(&self) -> Ref<DOMString> {`
	`55`	`+ self.return_value.borrow()`
	`56`	`+ }`
`52`	`57`	`}`
`53`	`58`
`54`	`59`	`impl BeforeUnloadEventMethods for BeforeUnloadEvent {`
Original file line number	Diff line number	Diff line change
`@@ -178,22 +178,15 @@ impl CompiledEventListener {`
`178`	`178`	`}`
`179`	`179`
`180`	`180`	`CommonEventHandler::BeforeUnloadEventHandler(ref handler) => {`
`181`		`- if let Some(event) = event.downcast::<BeforeUnloadEvent>() {`
`182`		`- let rv = event.ReturnValue();`
`183`		`-`
`184`		`- if let Ok(value) = handler.Call_(object,`
`185`		`- event.upcast::<Event>(),`
`186`		`- exception_handle) {`
`187`		`- match value {`
`188`		`- Some(value) => {`
`189`		`- if rv.is_empty() {`
`190`		`- event.SetReturnValue(value);`
`191`		`- }`
`192`		`- }`
`193`		`- None => {`
`194`		`- event.upcast::<Event>().PreventDefault();`
	`181`	`+ if let Ok(value) = handler.Call_(object, event, exception_handle) {`
	`182`	`+ if let Some(return_value) = value {`
	`183`	`+ if let Some(before_unload_event) = event.downcast::<BeforeUnloadEvent>() {`
	`184`	`+ if before_unload_event.return_value().is_empty() {`
	`185`	`+ before_unload_event.SetReturnValue(return_value);`
`195`	`186`	`}`
`196`	`187`	`}`
	`188`	`+ } else {`
	`189`	`+ event.PreventDefault();`
`197`	`190`	`}`
`198`	`191`	`}`
`199`	`192`	`}`