Opened 7 years ago

Closed 6 years ago

#16337 closed defect (fixed)

dojox/mobile: quickly touching/clicking two list items can trigger two view transitions

Reported by: Adrian Vasiliu Owned by: Eric Durocher
Priority: undecided Milestone: 1.8.4
Component: DojoX Mobile Version: 1.8.1
Keywords: Cc:
Blocked By: Blocking:

Description

Touching/clicking a dojox/mobile/ListItem with a moveTo specified triggers a view transition. Now, if the user touches/clicks one item then quickly touches/clicks another item, the transitions to both target views can be triggered. (This does not happen if transition:none is set on the list items.)

How to reproduce:

  1. Run the attached testTwoViewTransitions.html (placed in dojox/mobile/tests).
  2. Click/touch the item "View 1" then quicly quick/touch the item "View 2".

==> Both target views are made visible, which is clearly undesired. The same happens for the items marked "(bookmarkable)" (their moveTo specifies a bookmarkable target). On the contrary, it does not happen for the items marked "no transition effect" (they have transition:'none'), nor for items that have href specified instead of moveTo (the target of the item touched/clicked last wins).

Reproduced on both desktop (Chrome, FF) and mobile (Safari/iPhone) browsers.

Attachments (4)

testTwoViewTransitions.html (2.5 KB) - added by Adrian Vasiliu 7 years ago.
Test app for #16337
patch16337.patch (2.0 KB) - added by Adrian Vasiliu 7 years ago.
Do not trigger a view transition if one is already ongoing - Adrian Vasiliu, IBM, CCLA
patch16337-new.patch (1.2 KB) - added by Adrian Vasiliu 7 years ago.
Do not trigger a view transition if one is already ongoing (thanks ykami) - Adrian Vasiliu, IBM, CCLA
patch16337-new2.patch (2.7 KB) - added by Adrian Vasiliu 7 years ago.
Fix broken view after some cases of concurrent transitions, plus fix for compat mode - thanks ykami, IBM, CCLA

Download all attachments as: .zip

Change History (18)

Changed 7 years ago by Adrian Vasiliu

Attachment: testTwoViewTransitions.html added

Test app for #16337

comment:1 Changed 7 years ago by Adrian Vasiliu

This is not due to the "delayed selection" mechanism, it also happens if this is disabled.

Modifying _ItemBase to subscribe to "dojox/mobile/beforeTransitionOut" and "/dojox/mobile/afterTransitionIn" such that it can avoid triggering a new transition when one is already ongoing does fix it. This should also work when transitions are triggered by user's custom actions. However, we need to evaluate the impact on performance of the transition when many items are present, since they would all be notified twice per transition.

On the other side, this solution would ensure the later transition (for the item touched/clicked last) is not done, while it might be preferable that the later one wins. However it seems difficult to ensure the already started transition is correctly cancelled.

Another aspect is that this solution would imply that we critically depend on View to guarantee that a beforeTransition topic is always followed by a afterTransition topic. Any corner case when this wouldn't hold would result in items not triggering their action anymore... A more robust variant would be that items rely on the ViewController shared instance (the controller emits beforeTransition / afterTransition events but only to the item which initiates them). A newly introduced global flag "duringTransition" in the controller would be robust, and would avoid the performance impact of having each item subscribed to transition topics.

Last edited 7 years ago by Adrian Vasiliu (previous) (diff)

comment:2 Changed 7 years ago by Adrian Vasiliu

A completely different way (a brute-force way...) would be to not trigger item's action if any other's item action has been triggered less than some number of ms before... But we need a central place where this info can be managed for all items (which don't necessarily all have the same parent). The threshold would be very hard to chose. And doing so would prevent people for having custom actions which do not trigger view transitions and can be triggered one after the other quickly...

All in one, the key pain in this issue seem to be that the item triggers actions (ours, or user's, and potentially asynchronous) while it has no generic mean to know when this action completed.

Maybe a way out of it would be to not try to avoid triggering the second transition, and instead to ensure that the second view transition really "overrides" the effect of the first view transition. In our test case, if the second transition would win (instead of having both target views visible), the user wouldn't complain, I guess, even if the first transition executes for nothing.

Last edited 7 years ago by Adrian Vasiliu (previous) (diff)

comment:3 Changed 7 years ago by Eric Durocher

Component: GeneralDojoX Mobile
Owner: set to Eric Durocher

comment:4 Changed 7 years ago by Adrian Vasiliu

Just to add to the description of the bug:

On touch-enabled devices, the same misbehavior occurs if touching with different fingers several items at a time.

Changed 7 years ago by Adrian Vasiliu

Attachment: patch16337.patch added

Do not trigger a view transition if one is already ongoing - Adrian Vasiliu, IBM, CCLA

comment:5 Changed 7 years ago by Adrian Vasiliu

The attached patch aims to fix the issue at the central place: in ViewController. It relies on the assumption that the topic "/dojox/mobile/afterTransitionIn" always follows the topic "/dojox/mobile/beforeTransitionOut". The behavior is unmodified for items that open external views, which do not seem to require the fix.

We could additionally introduce in _ItemBase checks for the number of touches of the touch events, such that we don't react in case more than one finger touches (the same, or different items). However, this may be more fragile, and the above fix seems to make it unnecessary. On the other side, we may not want to forbid people from triggering the action with two fingers on the same item. Can be revisited. (Note that checking the number of touches would anyway not solve the issue for fast successive touches or clicks on different items.)

Changed 7 years ago by Adrian Vasiliu

Attachment: patch16337-new.patch added

Do not trigger a view transition if one is already ongoing (thanks ykami) - Adrian Vasiliu, IBM, CCLA

comment:6 Changed 7 years ago by Adrian Vasiliu

The attached patch16337-new.patch, suggested by ykami (thanks!), is a new, better variant of the fix. Differently than the initially proposed fix, it also covers the case of view transitions triggered by direct calls of View.performTransition, thus bypassing the ViewController.

comment:7 Changed 7 years ago by cjolif

Milestone: tbd1.8.2

comment:8 Changed 7 years ago by cjolif

In [30075]:

refs #16337. Do not trigger a view transition if one is already ongoing. Thanks Adrian Vasiliu & ykami, IBM, CCLA. !strict

comment:9 Changed 7 years ago by cjolif

In [30076]:

refs #16337. Do not trigger a view transition if one is already ongoing. Thanks Adrian Vasiliu & ykami, IBM, CCLA. !strict

comment:10 Changed 7 years ago by cjolif

Resolution: fixed
Status: newclosed

comment:11 Changed 7 years ago by Adrian Vasiliu

Unfortunately this ticket needs to be reopen. While testing the mobile gallery, Sebastien Brunot found troubles which appear to be side-effects of the fix of this ticket.

How to reproduce (in trunk and Dojo 1.8.2 or 1.8.3):

  1. Load http://demos.dojotoolkit.org/demos/mobileGallery/demo-iphone.html (on all browsers including Chrome on desktop).
  2. Select for instance "Swap View".
  3. Click/touch repeatedly and quickly the "Source" / "Demo" button.

==> After a number of click/touch, there is no view transition anymore.

Also:

  1. Load http://demos.dojotoolkit.org/demos/mobileGallery/demo-iphone.html
  2. Select "Buttons"
  3. Alternatively click/touch repeatedly and quickly on any two tabs of the TabBar (say, "Color" and "Texture").

==> After a number of click/touch, there is no view transition anymore.

comment:12 Changed 7 years ago by cjolif

Milestone: 1.8.21.8.4
Resolution: fixed
Status: closedreopened

Changed 7 years ago by Adrian Vasiliu

Attachment: patch16337-new2.patch added

Fix broken view after some cases of concurrent transitions, plus fix for compat mode - thanks ykami, IBM, CCLA

comment:13 Changed 6 years ago by cjolif

In [30432]:

refs #16337. Fix broken view after some cases of concurrent transitions + fix for compat mode. Thanks ykami & Adrian Vasiliu (IBM, CCLA). !strict.

comment:14 Changed 6 years ago by cjolif

Resolution: fixed
Status: reopenedclosed

In [30433]:

fixes #16337. Fix broken view after some cases of concurrent transitions + fix for compat mode (1.8 backport). Thanks ykami & Adrian Vasiliu (IBM, CCLA). !strict.

Note: See TracTickets for help on using tickets.