[webkit-reviews] review granted: [Bug 96480] REGRESSION(r128270): It made fast/events/popup-blocking-timers.html flakey : [Attachment 166686] Patch

Tue Oct 2 09:15:49 PDT 2012

Daniel Bates <dbates at webkit.org> has granted jochen at chromium.org's request for
review:
Bug 96480: REGRESSION(r128270): It made fast/events/popup-blocking-timers.html
flakey
https://bugs.webkit.org/show_bug.cgi?id=96480

Attachment 166686: Patch
https://bugs.webkit.org/attachment.cgi?id=166686&action=review

------- Additional Comments from Daniel Bates <dbates at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=166686&action=review

Looks sane to me.

> LayoutTests/fast/events/popup-blocking-timers1.html:31
> +	   function clickButton() {
> +	       var button = document.getElementById("test");
> +	       var buttonX = button.offsetLeft + button.offsetWidth / 2;
> +	       var buttonY = button.offsetTop + button.offsetHeight / 2;
> +	       if (window.eventSender) {
> +		   eventSender.mouseMoveTo(buttonX, buttonY);
> +		   eventSender.mouseDown();
> +		   eventSender.mouseUp();
> +	       }
> +	   }	    

I know that you derived this function from the function of the same name in the
original test. I take it that it's necessary to use EventSender to actually
simulate a mouse click of a person as opposed to calling click() on the HTML
button element?

> LayoutTests/fast/events/popup-blocking-timers2.html:1
> +<head>

Unless you intend to test quirks mode, I suggest that we include a DOCTYPE, say
<!DOCTYPE html>.

> LayoutTests/fast/events/popup-blocking-timers2.html:33
> +	   function clickButton() {
> +	       var button = document.getElementById("test");
> +	       var buttonX = button.offsetLeft + button.offsetWidth / 2;
> +	       var buttonY = button.offsetTop + button.offsetHeight / 2;
> +	       if (window.eventSender) {
> +		   eventSender.mouseMoveTo(buttonX, buttonY);
> +		   eventSender.mouseDown();
> +		   eventSender.mouseUp();
> +	       }
> +	   }	    

Ditto.

> LayoutTests/fast/events/popup-blocking-timers3.html:41
> +	   function clickButton() {
> +	       var button = document.getElementById("test");
> +	       var buttonX = button.offsetLeft + button.offsetWidth / 2;
> +	       var buttonY = button.offsetTop + button.offsetHeight / 2;
> +	       if (window.eventSender) {
> +		   eventSender.mouseMoveTo(buttonX, buttonY);
> +		   eventSender.mouseDown();
> +		   eventSender.mouseUp();
> +	       }
> +	   }	    

Ditto.

> LayoutTests/fast/events/popup-blocking-timers4.html:35
> +	   function clickButton() {
> +	       var button = document.getElementById("test");
> +	       var buttonX = button.offsetLeft + button.offsetWidth / 2;
> +	       var buttonY = button.offsetTop + button.offsetHeight / 2;
> +	       if (window.eventSender) {
> +		   eventSender.mouseMoveTo(buttonX, buttonY);
> +		   eventSender.mouseDown();
> +		   eventSender.mouseUp();
> +	       }
> +	   }	    

Ditto.

> LayoutTests/fast/events/popup-blocking-timers5.html:36
> +	   function clickButton() {
> +	       var button = document.getElementById("test");
> +	       var buttonX = button.offsetLeft + button.offsetWidth / 2;
> +	       var buttonY = button.offsetTop + button.offsetHeight / 2;
> +	       if (window.eventSender) {
> +		   eventSender.mouseMoveTo(buttonX, buttonY);
> +		   eventSender.mouseDown();
> +		   eventSender.mouseUp();
> +	       }
> +	   }	    

Ditto.

> LayoutTests/fast/events/popup-blocking-timers6.html:36
> +	   function clickButton() {
> +	       var button = document.getElementById("test");
> +	       var buttonX = button.offsetLeft + button.offsetWidth / 2;
> +	       var buttonY = button.offsetTop + button.offsetHeight / 2;
> +	       if (window.eventSender) {
> +		   eventSender.mouseMoveTo(buttonX, buttonY);
> +		   eventSender.mouseDown();
> +		   eventSender.mouseUp();
> +	       }
> +	   }	    

Ditto.