Opened 6 years ago

Last modified 6 years ago

#7876 new defect

Double clicking an Activity in the Home view, or Journal causes 2 instances to launch

Reported by: garycmartin Owned by: tomeu
Priority: blocker Milestone: 8.2.0 (was Update.2)
Component: sugar Version: Development build as of this date
Keywords: blocks-:8.2.0 polish:8.2.0 r+ Cc: ixo, gregorio, erikos
Blocked By: Blocking: #8090
Deployments affected: Action Needed: code
Verified: no

Description

Testing on Joyride 2296, double clicking an activity in the home ring causes 2 instances to launch. Double clicking used to be filtered, and add this to the now very slow latency to display the new launch splash screens (in some cases 5 seconds, it's activity dependant for some weird reason), I think this is a notable UI regression.

Example: Switch to home ring view; double click on write; note after some delay (and an almost missing launch animation due to the overload) that you now have 2 instances of Write in the frame.

Attachments (4)

0001-7876-Dont-launch-multiple-instances-on-double-click.patch (4.0 KB) - added by tomeu 6 years ago.
0002-7876-Dont-launch-multiple-instances-on-double-click.patch (4.9 KB) - added by erikos 6 years ago.
added the mesh view to the double click patch
0001-7876-journal-dont-launch-multiple-instances-on-double-click.patch (2.7 KB) - added by erikos 6 years ago.
patches for the expanded and the collapsentry
result-of-double-click-resume-from-fournal.jpg (82.1 KB) - added by garycmartin 6 years ago.
Example of what happens if you accidently double click an entry to resume from Journal (without erikos' 0001 Journal patch).

Download all attachments as: .zip

Change History (41)

comment:1 follow-up: Changed 6 years ago by Eben

This sounds like a deal breaker to me. Activity launch feedback needs to be perceivably instantaneous, and it needs to be impossible to launch two instances with a double-click.

comment:2 in reply to: ↑ 1 Changed 6 years ago by garycmartin

Replying to Eben:

This sounds like a deal breaker to me. Activity launch feedback needs to be perceivably instantaneous, and it needs to be impossible to launch two instances with a double-click.

Yes, it does tend to void the whole goal of the launching feedback. I posted a separate track #7742 a week ago about the latency regression, the allowing multi click makes it even worse (it's not just double clicks, I easily clicked and launched Turteart 4 times before the launch feedback started).

comment:3 Changed 6 years ago by ixo

  • Cc ixo added

comment:4 follow-up: Changed 6 years ago by gregorio

  • Keywords blocks:8.2.0 added; blocks?:8.2.0 removed

Hi Guys,

Does the second activity "hang" like it used to?

If so its definitely a blocker. In any case, I think its important enough to fix so I'll mark it must fix, but it may not survive the next triage so please resolve it ASAP if possible.

Thanks,

Greg S

comment:5 Changed 6 years ago by gregorio

  • Cc gregorio added

comment:6 follow-up: Changed 6 years ago by erikos

From my testing the second activity does not hang. To get to open two activities you really need to do a double click (clicking two times fast) - that behaviour I would only expect by users who are already used to a 'double-click' system. I personally never had this problem. First time I saw that this was 'possible' was when I tested this bug.

comment:7 in reply to: ↑ 4 ; follow-up: Changed 6 years ago by garycmartin

Replying to gregorio:

Does the second activity "hang" like it used to?

No, the 2 or more activities are both functional, the one case I know about that breaks is the Log activity where all its instances become borked.

comment:8 in reply to: ↑ 6 Changed 6 years ago by garycmartin

Replying to erikos:

To get to open two activities you really need to do a double click (clicking two times fast) - that behaviour I would only expect by users who are already used to a 'double-click' system. I personally never had this problem. First time I saw that this was 'possible' was when I tested this bug.

I notice just about everybody about me double click every darn thing under the sun (novice, short exposure Mac/Windows users). And the current click, and wait at least 1-2 seconds, to see any UI feedback is just too tempting for a kid who thinks their click may not have been registered. If the #7742 latency issue can be improved on, this track would be slightly less critical; however this is a regression from previous official releases. We'll be back getting complaints from teaches that kids are accidentally launching multiple activities, and activities are taking too long to start (because several are starting up at once).

This is a 'basic UI 101 class' behaviour, I'm amazed it's fallen out the UI again :-(

comment:9 in reply to: ↑ 7 Changed 6 years ago by erikos

Replying to garycmartin:

Replying to gregorio:

Does the second activity "hang" like it used to?

No, the 2 or more activities are both functional, the one case I know about that breaks is the Log activity where all its instances become borked.

the log problem was #7720

comment:10 Changed 6 years ago by marco

  • Action Needed changed from never set to design

comment:11 Changed 6 years ago by marco

  • Priority changed from high to blocker

comment:12 Changed 6 years ago by kimquirk

  • Keywords blocks?:8.2.0 added; blocks:8.2.0 removed
  • Priority changed from blocker to high

#7754 (opening two instances of browse causes problems) is fixed, so we think this is no longer a blocker.

comment:13 Changed 6 years ago by tomeu

Agreed with what Kim said about blocking.

About fixing this particular bug, I see two possibilities:

  1. Best fix: immediately show the launcher splash screen after the first click is received.
  2. Interim, easier fix: don't process clicks on activity icons for 100-500ms after the first click.

comment:14 Changed 6 years ago by tomeu

  • Owner changed from marco to tomeu

comment:15 Changed 6 years ago by jg

  • Keywords polish:8.2 added

Tomeu, seems like #2 is worth doing for 8.2. This problem is one of the ones seen heavily in the field....

comment:16 Changed 6 years ago by tomeu

  • Action Needed changed from design to review
  • Keywords r? added

comment:17 Changed 6 years ago by erikos

My first comment would be that we have some more places where you can launch an activity - the journal (at different places) and the mesh view. I guess we want to cover those as well?

comment:18 Changed 6 years ago by tomeu

Do we? I guess it depends on the frequency with which activities are launched from those other places.

Changed 6 years ago by erikos

added the mesh view to the double click patch

Changed 6 years ago by erikos

patches for the expanded and the collapsentry

comment:19 Changed 6 years ago by tomeu

  • Keywords r+ added; r? removed

Those patches look good to me, I guess we can set r+

comment:20 Changed 6 years ago by erikos

Since this is a crucial area I would like to see some testing of another person as well - garycmartin maybe you can have a go, that would be more than welcome?

comment:21 Changed 6 years ago by garycmartin

  • Action Needed changed from review to diagnose

Took some time to diagnose, but patch does not currently fix issue, sorry for the bad news. I've been using just 0001-7876-Dont-launch-multiple-instances-on-double-click.patch in my test/diagnose cycles (joyride-2301 XO B4).

Double, triple... quintuple clicks et al still all get through and launch multiple activity instances. The additional incoming clicks, after the first, appear to be delayed until after the launchwindow has appeared. Any additional click arrival times (however quickly made) are recorded well past the _CLICK_DELAY = 1 sec setting. Through trial and error, bumping _CLICK_DELAY up to 4 seems to be long enough to block additional delayed click events, though that's rather a large sledge hammer approach.

I'm not sure of the detail regarding event loop processing, so it's only a guess that the launchwindow might be delaying additional clicks after the first, could well be some other part of event loop processing.

comment:22 Changed 6 years ago by garycmartin

I thought I'd take a stab and try a different approach that stops the icon accepting clicks, however it exhibits the same issue as tomeu's patches and requires a ~4000ms delay, so I post it here incase it helps clarify what is happening for someone else. Sorry it's not in real patch format, I haven't mastered that foo bar ninja move yet (and I'm just working on a live XO set of source, rubber keyboard and all – future kid dev style):

/usr/share/sugar/shell/view/home/favoritesview.py
314         self._activity_info = activity_info
315         self._uncolor()
316         self.connect('hovering-changed', self.__hovering_changed_event_cb)
    -       self.connect('button-release-event', self.__button_release_event_cb)
317 +        self.__connect_button()
318         
319     def create_palette(self):
320         palette = ActivityPalette(self._activity_info)
...
340     def __button_release_event_cb(self, icon, event):
341         self.palette.popdown(immediate=True)
342         self._uncolor()
343         view.Shell.get_instance().start_activity(self._activity_info.bundle_id)
344 +       self.disconnect(self._release_handler_id)
345 +       gobject.timeout_add(4000, self.__connect_button)
346 
347 +   def __connect_button(self):
348 +       self._release_handler_id = self.connect('button-release-event',
349 +                                               self.__button_release_event_cb)

comment:23 Changed 6 years ago by erikos

For me it is not possible to produce the double click with the patches - maybe I am just doing something wrong :/

comment:24 Changed 6 years ago by garycmartin

OK, I will wipe, reapply and test again. Perhaps having debugging enabled is saturating things during a launch. Will go experiment some more.

comment:25 Changed 6 years ago by Eben

  • Blocking 8090 added

comment:26 Changed 6 years ago by marco

  • Keywords polish:8.2.0 added; polish:8.2 removed

comment:27 Changed 6 years ago by marco

  • Action Needed changed from diagnose to code

comment:28 Changed 6 years ago by cjb

  • Keywords blocks-:8.2.0 added; blocks?:8.2.0 removed

This looks like polish. We'll take it if it's ready soon.

comment:29 Changed 6 years ago by garycmartin

  • Priority changed from high to blocker

Just re-testing this on joyride-2444, still a problem. With no other activity load, I see a multi second delay (7sec in one case) before the launch window is displayed and any UI feedback is shown. Quite easy to launch 7 instances of TurtleArt, causing OOM and a lockup requiring a hard power off :-( Sorry, I'm going to raise this as a blocker as it's such a trivial way to crash Sugar.

comment:30 follow-up: Changed 6 years ago by marco

Do you have a lot entries in your journal? Is the feedback immediate if u test with a clean journal? Where are you launching the activity from?

comment:31 in reply to: ↑ 30 Changed 6 years ago by garycmartin

Replying to marco:

Do you have a lot entries in your journal? Is the feedback immediate if u test with a clean journal? Where are you launching the activity from?

Launching from home screen set to ring layout, with 22 fav's set (out of ~55 installed). Tested with both 750 Journal entries, and zero Journal entries. No other Activities running, clean reboot, extra debug logging disabled. Clicking TurtleArt-10, time before launch window starts to show 5-8sec. Also tested launching from the home list view, and as a resume from Journal, same 5-8sec range results.

comment:32 Changed 6 years ago by garycmartin

OK. Think I have it... Tested launching times while connected jabberd server (about ~10 buddies showing, maybe 2 visible activities share icons) and with jabberd server removed (just mesh icons and ~6 APs showing in Neighborhood). Allowing time for the buddy appearances to stabilise before testing when connected to jabberd (i.e no more visual buddy updates) gave the 5-8sec delay before launch window result. Removing the jabberd server setting, and restarting, the launch window appears 1-2sec after clicking.

With just one XO here I can't tell if it's jabberd vs salute, X number of buddies, N shared activities, or some combination that brings the launch window appearance time to it's knees. I'll try and keep an eye on the Neighborhood and report if I see a pattern.

Hope that helps others test.

comment:33 Changed 6 years ago by garycmartin

Sorry, above was for #7742 (I've added it there), got my trac windows mixed up.

comment:34 Changed 6 years ago by garycmartin

Just testing a clean build 8.2-765. Double clicking (a Journal entry, or an Activity on Home view) is still launching 2 instances :-( Lets hope kids don't double click.

I re-applied 0001-7876-journal-dont-launch-multiple-instances-on-double-click.patch and 0002-7876-Dont-launch-multiple-instances-on-double-click.patch on this latest build. Double clicking an Activity icon in Home view still launches two instances :-(. But double clicking in Journal is fixed!! You get one working instance, so no more nasty left over 'throbbers'! Any chance that the Journal patch at least can make it in to 8.2??

comment:35 Changed 6 years ago by skierpage

I see this with Paint and Memorize in 8.2-763

If I double-click Paint , I get two icons in the frame, one of which has a doubled menu ("Paint Activity - Resume - Stop - Resume - Stop").

If I click Memorize three times, I get three activities and two gray circles with Resume-Stop menus.

comment:36 Changed 6 years ago by garycmartin

  • Cc erikos added

Was hoping at least 0001-7876-journal-dont-launch-multiple-instances-on-double-click.patch was going to make it into 8.2, as this is a big improvement for accidental Journal double click resumes. Journal is the worst behaved currently as only the first resume is successful, the second is left as a 'throbber', both get their Frame menus screwed up, and when you do finally get the working one quit, you end up staring at the launcher window of the one that borked out (you have to manually switch back to home). This all goes away with patch 0001.

Just re-tested the patch on build 8.2-766 and Journal item double-clicks correctly result in the resumption of just one working Activity instance.

Changed 6 years ago by garycmartin

Example of what happens if you accidently double click an entry to resume from Journal (without erikos' 0001 Journal patch).

comment:37 Changed 6 years ago by garycmartin

  • Summary changed from Double clicking an activity in the home ring causes 2 instances to launch to Double clicking an Activity in the Home view, or Journal causes 2 instances to launch

Just editing the summary to be explicit that click de-bouncing is an issue for both Journal and Home view.

Note: See TracTickets for help on using tickets.