Merge lp:~er-abhinav-upadhyay/ubuntu/natty/apport/bugfix-357847 into lp:~ubuntu-core-dev/ubuntu/natty/apport/ubuntu

[Abhinav Upadhyay]

I have added support for -w/--window option which will enable user to point to the problematic window and generate a crash report for that application.

I used Matt Zimmerman's implementation of the idea, as it was being discussed on the bug report page.

[Dustin Kirkland  (kirkland)]

Per feedback from pitti in IRC, he would rather review this one himself. I'm stepping aside now.

[Martin Pitt (pitti)]

Note that the shell wrapper isn't supposed to do any option processing. Support for this should be added to apport/ui.py instead (where it can also be covered by test cases, at least the ones which check CLI processing).

review: Needs Fixing

[Abhinav Upadhyay (er-abhinav-upadhyay)]

> Note that the shell wrapper isn't supposed to do any option processing.
> Support for this should be added to apport/ui.py instead (where it can also be
> covered by test cases, at least the ones which check CLI processing).

Ok. Thanks for reviewing it :-)

I will try to implement this in apport/ui.py . I was wondering if -x option is ok ? I chose -x because it invokes xprop.

[Martin Pitt (pitti)]

Hello Abhinav,

Abhinav Upadhyay [2011-03-14 17:33 -0000]:
> I will try to implement this in apport/ui.py . I was wondering if -x
> option is ok ? I chose -x because it invokes xprop.

It shouldn't be named after the implementation, but after the
functionality. What do you think about -w/--window?

Thanks!

Martin

--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)

[lp:~er-abhinav-upadhyay/ubuntu/natty/apport/bugfix-357847]

1760. By Abhinav Upadhyay on 2011-03-17

apport/ui.py: Modified to add support for -w/--window option which will let the user
point and click at the buggy window, to generate the crash report for that application.

1761. By Abhinav Upadhyay on 2011-03-17

apport/ui.py: Added support for -w/--window option which will enable user to select the buggy window.
Also updated the debian changelog

[Martin Pitt (pitti)]

Hey Abhinav,

thanks for your work on that!

Unfortunately you created a MP against the Ubuntu packaging branch instead of trunk, so I cannot formally merge this. I applied the patch to trunk:

  http://bazaar.launchpad.net/~apport-hackers/apport/trunk/revision/1872

I also cleaned it up a little bit, fixed the tests to succeed, avoid using the shell, fix i18n, etc.

review: Approve

=== modified file 'apport/ui.py'
--- apport/ui.py	2011-02-28 11:07:58 +0000
+++ apport/ui.py	2011-03-17 12:19:31 +0000
@@ -585,6 +585,15 @@
             except OSError as e:
                 self.ui_error_message(_('Invalid problem report'), excstr(e))
             return True
+        elif self.options.get_window_pid == True:
+        	self.ui_info_message('Apport --Window', 'After closing this message '\
+        	'box please point and click on the buggy window to report the problem')
+	        pid = subprocess.Popen(['xprop _NET_WM_PID | cut -d= -f2'], \
+	        shell = True, stdout = subprocess.PIPE).communicate()[0]
+	        pid = int(pid.decode('UTF-8').strip())
+	        self.options.pid = pid
+	        self.options.get_window_pid = False
+	        return self.run_report_bug()
         else:
             return self.run_crashes()
 
@@ -651,6 +660,8 @@
             help=_('Add an extra tag to the report. Can be specified multiple times.'))
         optparser.add_option('-v', '--version', action='store_true',
             help=_('Print the Apport version number.'))
+        optparser.add_option('-w', '--window', action='store_true', dest='get_window_pid', default=False,
+        	help=_('Point the mouse pointer at the buggy window and file a bug report'))
 
         if len(sys.argv) > 0 and cmd.endswith('-bug'):
             for o in ('-f', '-u', '-s', '-p', '-P', '-c'):
@@ -2679,14 +2690,16 @@
             # no arguments -> show pending crashes
             _chk('apport-gtk', None, {'filebug': False, 'package': None,
                 'pid': None, 'crash_file': None, 'symptom': None, 
-                'update_report': None, 'save': None, 'tag': []})
+                'update_report': None, 'save': None, 'get_window_pid': False, 
+                'tag': []})
             # updating report not allowed without args
             self.assertRaises(SystemExit, _chk, 'apport-collect', None, {})
 
             # package 
             _chk('apport-kde', 'coreutils', {'filebug': True, 'package':
                 'coreutils', 'pid': None, 'crash_file': None, 'symptom': None, 
-                'update_report': None, 'save': None, 'tag': []})
+                'update_report': None, 'save': None, 'get_window_pid': False, 
+                'tag': []})
 
             # symptom is preferred over package
             f = open(os.path.join(symptom_script_dir, 'coreutils.py'), 'w')
@@ -2697,33 +2710,37 @@
             f.close()
             _chk('apport-cli', 'coreutils', {'filebug': True, 'package': None,
                  'pid': None, 'crash_file': None, 'symptom': 'coreutils',
-                 'update_report': None, 'save': None, 'tag': []})
+                 'update_report': None, 'save': None, 'get_window_pid': False, 
+                 'tag': []})
 
             # PID
             _chk('apport-cli', '1234', {'filebug': True, 'package': None,
                  'pid': '1234', 'crash_file': None, 'symptom': None,
-                 'update_report': None, 'save': None, 'tag': []})
+                 'update_report': None, 'save': None, 'get_window_pid': False, 
+                 'tag': []})
 
             # .crash/.apport files; check correct handling of spaces
             for suffix in ('.crash', '.apport'):
                 _chk('apport-cli', '/tmp/f oo' + suffix, {'filebug': False,
                      'package': None, 'pid': None, 
                      'crash_file': '/tmp/f oo' + suffix, 'symptom': None,
-                     'update_report': None, 'save': None, 'tag': []})
+                     'update_report': None, 'save': None, 'get_window_pid': False, 
+                     'tag': []})
 
             # executable
             _chk('apport-cli', '/usr/bin/tail', {'filebug': True, 
                  'package': 'coreutils',
                  'pid': None, 'crash_file': None, 'symptom': None, 
-                 'update_report': None, 'save': None, 'tag': []})
+                 'update_report': None, 'save': None, 'get_window_pid': False, 
+                 'tag': []})
 
             # update existing report
             _chk('apport-collect', '1234', {'filebug': False, 'package': None,
                  'crash_file': None, 'symptom': None, 'update_report': 1234,
-                 'tag': []})
+                 'get_window_pid': False,'tag': []})
             _chk('apport-update-bug', '1234', {'filebug': False, 'package': None,
                  'crash_file': None, 'symptom': None, 'update_report': 1234,
-                 'tag': []})
+                 'get_window_pid': False, 'tag': []})
 
         def test_parse_argv_apport_bug(self):
             '''parse_args() option inference when invoked as *-bug'''
@@ -2746,7 +2763,8 @@
             #
             _chk([], {'filebug': True, 'package': None,
                 'pid': None, 'crash_file': None, 'symptom': None, 
-                'update_report': None, 'save': None, 'tag': []})
+                'update_report': None, 'save': None, 'get_window_pid': None,
+                'get_window_pid': False, 'tag': []})
 
             #
             # single arguments
@@ -2755,7 +2773,8 @@
             # package
             _chk(['coreutils'], {'filebug': True, 'package':
                 'coreutils', 'pid': None, 'crash_file': None, 'symptom': None, 
-                'update_report': None, 'save': None, 'tag': []})
+                'update_report': None, 'save': None, 'get_window_pid': None, 
+                'get_window_pid': False, 'tag': []})
 
             # symptom (preferred over package)
             f = open(os.path.join(symptom_script_dir, 'coreutils.py'), 'w')
@@ -2766,25 +2785,29 @@
             f.close()
             _chk(['coreutils'], {'filebug': True, 'package': None,
                  'pid': None, 'crash_file': None, 'symptom': 'coreutils',
-                 'update_report': None, 'save': None, 'tag': []})
+                 'update_report': None, 'save': None, 'get_window_pid': False, 
+                 'tag': []})
             os.unlink(os.path.join(symptom_script_dir, 'coreutils.py'))
 
             # PID
             _chk(['1234'], {'filebug': True, 'package': None,
                  'pid': '1234', 'crash_file': None, 'symptom': None,
-                 'update_report': None, 'save': None, 'tag': []})
+                 'update_report': None, 'save': None, 'get_window_pid': False, 
+                 'tag': []})
 
             # .crash/.apport files; check correct handling of spaces
             for suffix in ('.crash', '.apport'):
                 _chk(['/tmp/f oo' + suffix], {'filebug': False,
                      'package': None, 'pid': None, 
                      'crash_file': '/tmp/f oo' + suffix, 'symptom': None,
-                     'update_report': None, 'save': None, 'tag': []})
+                     'update_report': None, 'save': None, 'get_window_pid': False, 
+                     'tag': []})
 
             # executable name
             _chk(['/usr/bin/tail'], {'filebug': True, 'package': 'coreutils',
                  'pid': None, 'crash_file': None, 'symptom': None, 
-                 'update_report': None, 'save': None, 'tag': []})
+                 'update_report': None, 'save': None, 'get_window_pid': False, 
+                 'tag': []})
 
             #
             # supported options
@@ -2794,17 +2817,17 @@
             _chk(['--save', 'foo.apport', 'coreutils'], {'filebug': True,
                 'package': 'coreutils', 'pid': None, 'crash_file': None,
                 'symptom': None, 'update_report': None, 'save': 'foo.apport',
-                'tag': []})
+                'get_window_pid': False, 'tag': []})
 
             # --tag
             _chk(['--tag', 'foo', 'coreutils'], {'filebug': True,
                 'package': 'coreutils', 'pid': None, 'crash_file': None,
                 'symptom': None, 'update_report': None, 'save': None,
-                'tag': ['foo']})
+                'get_window_pid': False, 'tag': ['foo']})
             _chk(['--tag', 'foo', '--tag', 'bar', 'coreutils'], {
                 'filebug': True, 'package': 'coreutils', 'pid': None,
                 'crash_file': None, 'symptom': None, 'update_report': None,
-                'save': None, 'tag': ['foo', 'bar']})
+                'save': None, 'get_window_pid': False, 'tag': ['foo', 'bar']})
 
     unittest.main()
 

=== modified file 'debian/changelog'
--- debian/changelog	2011-03-09 18:21:26 +0000
+++ debian/changelog	2011-03-17 12:19:31 +0000
@@ -1,3 +1,11 @@
+apport (1.19-0ubuntu4) natty; urgency=low
+
+  * apport/ui.py: Added support for -w/--window option which will enable user to select the 
+    problematic window and generate a crash report.
+    (LP: #357847)
+
+ -- Abhinav Upadhyay <[e-mail]>  Thu, 17 Mar 2011 17:42:16 +0530
+
 apport (1.19-0ubuntu3) natty; urgency=low
 
   * Merge fixes from trunk: