Code review comment for lp:~brandontschaefer/unity/unity.fix-middle-paste-reg

Revision history for this message
Thomi Richards (thomir-deactivatedaccount) wrote :

Hi,

First, the standard for clipboard is here: http://www.freedesktop.org/wiki/Specifications/ClipboardsWiki?action=show&redirect=Standards%2FClipboardsWiki

The PRIMARY clipboard should be set when the MOUSE selects some text, and should be pasted on MOUSE middle-click ONLY.
The CLIPBOARD clipboard is used for menu entries, and Ctrl+[cxv] shortcuts.

Regarding your first test:

50 + def test_paste_crash(self):
51 + """ This test is for this bug:926793 """
52 + self.dash.ensure_hidden()
53 + self.dash.toggle_reveal()
54 +
55 + kb = Keyboard();
56 + kb.type("SegFault")
57 + sleep(1)
58 +
59 + kb.press_and_release("Ctrl+a")
60 + kb.press_and_release("Ctrl+c")
61 + kb.press_and_release("Ctrl+v")
62 +
63 + searchbar = self.dash.get_searchbar()
64 + self.assertEqual(searchbar.search_string, u'SegFault')

This needs to be fixed, for a couple of reasons. First, we don't have a test that verifies that Ctrl+a works. So your test should be something like this:

50 + def test_paste_crash(self):
51 + """ This test is for this bug:926793 """
52 + self.dash.ensure_hidden()
53 + self.dash.toggle_reveal()
54 +
55 + kb = Keyboard();
56 + kb.type("SegFault")
57 + sleep(1)
58 +
59 + kb.press_and_release("Ctrl+a")
   kb.press_and_release("Delete")
63 + searchbar = self.dash.get_searchbar()
64 + self.assertEqual(searchbar.search_string, u'')

Secondly, you need to verify that Ctrl+C actually does something. If both Ctrl+C and Ctrl+V stopped working, your test would still pass. So how about somethingt like this:

50 + def test_paste_crash(self):
51 + """ This test is for this bug:926793 """
52 + self.dash.ensure_hidden()
53 + self.dash.toggle_reveal()
54 +
55 + kb = Keyboard();
56 + kb.type("SegFault")
57 + sleep(1)
58 +
59 + kb.press_and_release("Ctrl+a")
60 + kb.press_and_release("Ctrl+c")
60 + kb.press_and_release("Ctrl+v")
60 + kb.press_and_release("Ctrl+v")
62 +
63 + searchbar = self.dash.get_searchbar()
64 + self.assertEqual(searchbar.search_string, u'SegFaultSegfault')

We also need to test Ctrl+x (I assume we're supposed to support that):

50 + def test_paste_crash(self):
51 + """ This test is for this bug:926793 """
52 + self.dash.ensure_hidden()
53 + self.dash.toggle_reveal()
54 +
55 + kb = Keyboard();
56 + kb.type("SegFault")
57 + sleep(1)
58 +
59 + kb.press_and_release("Ctrl+a")
60 + kb.press_and_release("Ctrl+x")
62 +
63 + searchbar = self.dash.get_searchbar()
64 + self.assertEqual(searchbar.search_string, u'')

...and also:

50 + def test_paste_crash(self):
51 + """ This test is for this bug:926793 """
52 + self.dash.ensure_hidden()
53 + self.dash.toggle_reveal()
54 +
55 + kb = Keyboard();
56 + kb.type("SegFault")
57 + sleep(1)
58 +
59 + kb.press_and_release("Ctrl+a")
60 + kb.press_and_release("Ctrl+x")
60 + kb.press_and_release("Ctrl+v")
60 + kb.press_and_release("Ctrl+v")
62 +
63 + searchbar = self.dash.get_searchbar()
64 + self.assertEqual(searchbar.search_string, u'SegFaultSegfault')

Finally, we need to do something similar with the PRIMARY clipboard.

You can put something onto the clipboard artificiall with this code:

from gtk import Clipboard
cb = Clipboard(selection="PRIMARY") # can also be "CLIPBOARD", but we can test that easily enough as it is
cb.set_text("Hello World")

.. at this point, middle mouse clicking will paste "Hello World"

More info here:

http://developer.gnome.org/pygtk/stable/class-gtkclipboard.html

review: Needs Fixing

« Back to merge proposal