Btw, this refactoring will help to port that code to webkit2

Review of attachment 218008 [details] [review]:

Great cleanup in general, a few comments.

::: src/ephy-window.c
@@ +2108,3 @@
+	gboolean retval = FALSE;
+
+	if ((event->state & GDK_SHIFT_MASK) != GDK_SHIFT_MASK)

The original code checked that control was not pressed. Why did you drop it?

@@ +2136,3 @@
+	{
+		return retval;
+	}

Seems cleaner to have the rest of the method inside if (location) {} ... no point in having an early return so late ;)

@@ +2197,1 @@
 	}

I think you can just merge these two...

(In reply to comment #2)
> Review of attachment 218008 [details] [review]:
> 
> Great cleanup in general, a few comments.
> 
> ::: src/ephy-window.c
> @@ +2108,3 @@
> +    gboolean retval = FALSE;
> +
> +    if ((event->state & GDK_SHIFT_MASK) != GDK_SHIFT_MASK)
> 
> The original code checked that control was not pressed. Why did you drop it?

Because it wasn't needed, we are not checking whether the flag is present, we are checking whether the result is exactly the mask or not.

> @@ +2136,3 @@
> +    {
> +        return retval;
> +    }
> 
> Seems cleaner to have the rest of the method inside if (location) {} ... no
> point in having an early return so late ;)

Ok.

> @@ +2197,1 @@
>      }
> 
> I think you can just merge these two...

Ok.

Created attachment 218073 [details] [review]
Updated patch

Review of attachment 218073 [details] [review]:

You are totally right about the shift/control thing of course. Looks great!