Opened 10 years ago

Closed 10 years ago

Last modified 6 years ago

#1447 closed enhancement (fixed)

Display a message when an activity fails to start

Reported by: wadeb Owned by: alsroot
Priority: Normal Milestone:
Component: Sugar Version: Git as of bugdate
Severity: Unspecified Keywords: r? olpc-0.84
Cc: mstone, sascha_silbe, tomeu, erikos, bernie Distribution/OS: Unspecified
Bug Status: Unconfirmed

Description

The attached patch displays a simple error message when an activity's process ends while the activity is still flagged as launching.

Attachments (7)

0001-Display-error-message-when-activity-fails-to-launch.patch (6.5 KB) - added by wadeb 10 years ago.
0001-Notify-shell-when-activity-process-terminates.2.patch (1.9 KB) - added by wadeb 10 years ago.
Better patch for GIL lock issue
0001-Notify-shell-when-activity-process-terminates.patch (3.6 KB) - added by wadeb 10 years ago.
Update to GIL fix; remove debug prints
0001-Added-Close-button-to-activity-launcher-rewrote-lay.patch (14.2 KB) - added by wadeb 10 years ago.
New version of the Sugar patch
activity-failed-to-start.png (14.0 KB) - added by bernie 10 years ago.
Screenshot of the UI change introduced by this patch
sugar-toolkit-1447.patch (2.9 KB) - added by alsroot 10 years ago.
sugar-1447.patch (4.0 KB) - added by alsroot 10 years ago.

Download all attachments as: .zip

Change History (32)

comment:1 Changed 10 years ago by wadeb

Note: These patches should be considered a prototype for further development. I get occasional segfaults when pressing the Close button, and I would like to improve the looks further.

Also, if http://wiki.sugarlabs.org/go/Features/Problem_Reports is accepted, there needs to be a button to open the Report a problem control panel.

comment:2 Changed 10 years ago by mstone

These patches look promising to me and I'll be happy to help you iron out the wrinkles some other morning or evening. One suggestion in the mean-time: in addition to popping up a window, it's really important to the overall interaction design that we make a journal entry containing the activity's log file when activity launches fail because we can then bring existing tools into play in order to get that log file home. Might you be willing to write up a third patch containing this extension for me to test while I've got this stuff paged in to my head?

Lastly, regarding segfaults: take a look at #1123. I mention it because alsroot's patches in it caught my eye when I looked at the history of the activityfactory code that you're modifying.

comment:3 Changed 10 years ago by mstone

  • Cc mstone added

comment:4 Changed 10 years ago by sascha_silbe

  • Cc sascha_silbe added

comment:5 Changed 10 years ago by wadeb

#1123 seems very relevant to this bug. Here is the backtrace from the segfault for reference.

#0  PyErr_Fetch (p_type=0xbf93d39c, p_value=0xbf93d398, p_traceback=0xbf93d394) at Python/errors.c:235
#1  0x00b1254f in dbus_py_variant_level_clear (self=0x9b2df80) at abstract.c:143
#2  0x00b125f2 in DBusPyStrBase_tp_dealloc (self=0x9b2df80) at abstract.c:443
#3  0x02064e34 in dict_dealloc (mp=0x9b341c4) at Objects/dictobject.c:911
#4  0x02088155 in subtype_dealloc (self=0x9b2df6c) at Objects/typeobject.c:1006
#5  0x02064e34 in dict_dealloc (mp=0x9b3402c) at Objects/dictobject.c:911
#6  0x02088155 in subtype_dealloc (self=0x9a75b2c) at Objects/typeobject.c:1006
#7  0x0207daa7 in tupledealloc (op=0x968e92c) at Objects/tupleobject.c:170
#8  0x00df7121 in pyglib_error_check () from /usr/lib/python2.6/site-packages/gtk-2.0/glib/_glib.so
#9  0x002cd984 in g_source_callback_unref (cb_data=0x9a283c0) at gmain.c:941
#10 0x002cdf4e in g_source_destroy_internal (source=0x9adbbd0, context=<value optimized out>, have_lock=1) at gmain.c:720
#11 0x002ce35b in g_main_dispatch (context=<value optimized out>) at gmain.c:1849
#12 IA__g_main_context_dispatch (context=<value optimized out>) at gmain.c:2377
#13 0x002d19e0 in g_main_context_iterate (context=0x90a2f80, block=<value optimized out>, dispatch=1, self=0x90a4e98)
    at gmain.c:2455
#14 0x002d1e4f in IA__g_main_loop_run (loop=0x93f4e78) at gmain.c:2663
#15 0x070e7c09 in IA__gtk_main () at gtkmain.c:1205

Changed 10 years ago by wadeb

Better patch for GIL lock issue

comment:6 Changed 10 years ago by wadeb

Ok, this patch appears to fix the problem.

I basically punted and moved all the processing into an idle callback. This seems like the safest way to work around the GIL issue reported in #1123; this patch should be considered a more complete fix for that bug.

comment:7 follow-up: Changed 10 years ago by wadeb

  • Keywords r? added
  • Owner changed from tomeu to wadeb
  • Status changed from new to accepted

comment:8 in reply to: ↑ 7 Changed 10 years ago by wadeb

Replying to wadeb:

Doh, looks like I uploaded the wrong patch last night. I'm going to make some UI improvements as well, so removing r? for now.

comment:9 Changed 10 years ago by wadeb

  • Keywords r? removed

Changed 10 years ago by wadeb

Update to GIL fix; remove debug prints

Changed 10 years ago by wadeb

New version of the Sugar patch

comment:10 Changed 10 years ago by wadeb

I posted a brand new patch for Sugar.

  • Improves the layout so the icon doesn't move when the error appears
  • Uses Stop as suggested by Gary
  • Delays for a few seconds between pulses to give the user something to count and potentially improve performance.

comment:11 follow-up: Changed 10 years ago by bernie

  • Cc tomeu erikos added

Ping? I've tested this patch for a while, and I think it's a huge usability improvement. Can we review it and possibly merge it?

comment:12 in reply to: ↑ 11 Changed 10 years ago by garycmartin

Replying to bernie:

Ping? I've tested this patch for a while, and I think it's a huge usability improvement. Can we review it and possibly merge it?

A large +1 from me, it is a _HUGE_ improvement in behaviour. Now, FWIW I've seen a few spurious activity launches that leave duplicate top frame activity icons in the toolbar, but I've not been convinced that it is the result of this patch, yet.

comment:13 Changed 10 years ago by bernie

  • Cc bernie added

comment:14 Changed 10 years ago by tomeu

  • Keywords r? added

comment:15 Changed 10 years ago by bernie

YAY!

Changed 10 years ago by bernie

Screenshot of the UI change introduced by this patch

comment:16 Changed 10 years ago by tomeu

  • Milestone changed from 0.88 to 0.90

missed feature freeze

comment:17 Changed 10 years ago by alsroot

  • Keywords olpc-0.84 added
  • Owner changed from wadeb to alsroot
  • Status changed from accepted to assigned

I guess everybody forgot about it..

In my mind patch could be less invasive e.g. w/o introduction new new dbus method and most invasive part would be "Stop" button.

I'm preparing another patch that at least could useful for 0.84 branch and if everybody will agree, for 0.88

comment:18 Changed 10 years ago by alsroot

  • Keywords r! added; r? removed

comment:19 Changed 10 years ago by alsroot

sugar-toolkit patch is reusing NotifyLaunchFailure dbus method to notify shell about not successful activity termination (activity could already show main window) but sugar part will proper handle this. At the end wadeb's new dbus method will make code cleaner, I just not sure can we introduce new dbus API changes for 0.88 and for 0.84 (for XO-1.5).

sugar patch is based on wabeb's with just removing NotifyActivityEnded dbus call. If GUI part is a blocker for inclusion to 0.88, I guess it could be simplified to just showing "Stop" button.

Changed 10 years ago by alsroot

comment:20 Changed 10 years ago by alsroot

  • Keywords r? added; r! removed
  • Milestone changed from 0.90 to 0.88

Changed 10 years ago by alsroot

comment:21 Changed 10 years ago by alsroot

..if last patch could be considered as less invasive
it also adds new gettext string since w/o message stop button could be confusing

comment:23 Changed 10 years ago by bernie

Thank you, this is very useful!

comment:24 Changed 10 years ago by bernie

Sayamindu, can we backport this patch to 0.84?

comment:25 Changed 6 years ago by dnarvaez

  • Milestone 0.88 deleted

Milestone 0.88 deleted

Note: See TracTickets for help on using tickets.