Opened 11 years ago

Closed 11 years ago

Last modified 7 years ago

#1775 closed defect (fixed)

view source functionality should be more robust against buggy activities

Reported by: rgs Owned by: tomeu
Priority: Unspecified by Maintainer Milestone:
Component: Sugar Version: Git as of bugdate
Severity: Major Keywords: r+, robust file existence check
Cc: walter Distribution/OS: Unspecified
Bug Status: New

Description

Scenario:

We added view-source support to TurtleArt (see attached code snippet). I manage to hit a code path (a race condition somewhere or problem accessing the temp file) in which I got the following exception (in ~/.sugar/default/logs/shell.log):

File "/usr/lib/python2.6/dist-packages/jarabe/view/viewsource.py",  line 291, in __init__
 activity_button.props.group = document_button
 UnboundLocalError: local variable 'document_button' referenced before assignment

Checking the constructor of the Toolbar class in jarabe/view/viewsource.py it seems like the following test:

 if document_path is not None:

forgets to check file existence as the previous conditional does:

 if document_path is not None and os.path.exists(document_path)

The attached patch proposes a fix by starting document_button to None which seems cheaper then testing for the existence of a file twice.

Attachments (2)

ta-view-source-support.py (1.1 KB) - added by rgs 11 years ago.
protect-view-source.patch (934 bytes) - added by rgs 11 years ago.

Download all attachments as: .zip

Change History (7)

Changed 11 years ago by rgs

Changed 11 years ago by rgs

comment:1 Changed 11 years ago by erikos

  • Bug Status changed from Unconfirmed to New
  • Keywords r? added
  • Milestone changed from Unspecified by Release Team to 0.88
  • Severity changed from Unspecified to Major

Thanks for submitting the patch! I marked the ticket 'r?', as I guess you want to add it to the review queue. ( http://wiki.sugarlabs.org/go/Development_Team/Code_Review )

comment:2 Changed 11 years ago by tomeu

  • Keywords r+ added; r? removed

Looks great to me, thanks!

Simon, can you add Raul to the Sugar committers?

comment:3 Changed 11 years ago by erikos

I have added Raul to the Sugar committers. And pushed the patch:
http://git.sugarlabs.org/projects/sugar/repos/mainline/commits/da1be6e637b636f16e476a00b07e64164609c1a9

Thanks Raul!

comment:4 Changed 11 years ago by erikos

  • Resolution set to fixed
  • Status changed from new to closed

comment:5 Changed 7 years ago by dnarvaez

  • Milestone 0.88 deleted

Milestone 0.88 deleted

Note: See TracTickets for help on using tickets.