search

sugarlabs / infoslicer

GNU General Public License v2.0
2 stars 10 forks source link

Fix traceback caused due to bool return instead of an object #30

Closed nswarup14 closed 5 years ago

nswarup14 commented 5 years ago

Fixes #23

Referred https://github.com/sugarlabs/chat/commit/753d6f6b9f8ea65ad879a310ba1520fb61bdbe9b and https://github.com/sugarlabs/infoslicer/commit/4d96b5247f0a10a9eb9478490c95a51449f3b18a

Please review.

Aniket21mathur commented 5 years ago

@nswarup14 by making these changes are we assuming that mouseClickPositionIter[0] always exists i.e if not mouseClickPositionIter[0] always returns false? Thanks!

quozl commented 5 years ago

I don't understand why you are making this change. It is in code for handling previous versions of GTK. Given https://github.com/sugarlabs/infoslicer/commit/4d96b5247f0a10a9eb9478490c95a51449f3b18a and https://github.com/sugarlabs/infoslicer/commit/f0b503895527d1e384bbb357d5a31dbfe778d300 please provide more explanation for the change, and which version of GTK you have reproduced the problem in. Thanks.

nswarup14 commented 5 years ago

@quozl @Aniket21mathur I spent a while trying to understand why the traceback was being caused.

Against master, one way to reproduce traceback is 1) Run activity, default article appears 2) Try to select text from the article via mouse drag operation. Works fine if the drag operation is done over text. 3) Try the same operation over the blank spaces between paragraphs. Traceback is caused.

The reason being, as the docs say, if the position is over text, the tuple is returned with (True, iter), else (False, iter). However, for our purpose, the bool value is insignificant.

Another reproducer 1) Run activity 2) Switch to the Edit view, by clicking the Edit button on the toolbar 3) Try to drag content from left to right 4) Traceback is caused

This is because, there is an extra function defined in Editable_textbox.py which same as the one in Textbox.py. Also since (False, iter) is being returned, because the mouse position where the selection is being dropped has no text, the traceback is caused.

Kindly review and test.

Tested on Sugar v0.114, Ubuntu 18.04 and Gtk(3.22.30)

Aniket21mathur commented 5 years ago

Reviewed and Tested. Looks good to me. Thanks! Waiting for @quozl to review.