Closed Bug 1022755 Opened 6 years ago Closed 5 years ago
Possible race in the SMS navigation code
STR: * load a reference-workload-medium on the device (tested on a flame) * open the sms app * open the first thread at the top of the list (MMS) * press the back button Expected: * we go back to the thread list Actual: * the thread list is displayed for a brief instant and then we're stuck on an empty conversation panel with the header from the last conversation * Pressing the back button doesn't do anything
I saw this intermittently without finding a good STR, so thanks for this. I expect this is a regression of bug 881469, hence the nomination.
Juliene I guess his one will be fall from this sprint isnt?
It's not committed for this sprint but I planned to have a look if I have time before the end of this week if I find time. Otherwise it will wait for next sprint, yep.
I looked at it with Etienne, and it's not 100% reproducible for him either, so that's "good". Looks like the issue is in Navigation.slide: we might have several transitionend happening when we press "back" quickly after entering a thread. However, we're not supposed to start a new slide before the panel transition is fully finished. Therefore, I wonder if we don't get a transitionend event from "somewhere" else instead. Maybe we should check for event.target and event.propertyName in the transitionend event handling. I think the next step would be to log stuff in  and then reproduce.  https://github.com/mozilla-b2g/gaia/blob/82e957160ca017087bd374cd051339e55b641e68/apps/sms/js/navigation.js#L298
Assignee: nobody → azasypkin
Status: NEW → ASSIGNED
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → All
Hey Julien, As we discussed on IRC, I ni? you so that you can help with investigation if you have time. Some summary of what I've already investigated: 1. This issues in fact is not regression from bug 881469, it just became more noticeable after it. I can reproduce root problem on Flame v1.4, it just looks better; 2. Issue can be easily reproduced on Flame (master), with single thread (1 sms and 1 mms inside) by quickly tapping on Back button right after navigating to Thread panel; 3. Panels behaviour objects are initialized\deinitialized correctly, "data-position" attribute on wrapper element is correct, but "translateX(...)" is stuck while transitioning; 4. Some crazy hacky ways that may be useful to know and help to get rid of the problem (but bring another problems though): 4.1 make ".messages-date-group" as inline or 4.2 use setTimeout(.., 0)\requestAnimationFrame in Navigation.slide method; 5. And just in case: once transition between panels is stuck, it can be restored via toggling "position" property for ".panel" class in AppManager. Thanks!
One more thing that I've just noticed, quickly pressing Back button once you enter message draft sometime removes that draft from inbox (until you close and open SMS app once again).
So, the issue happens when we have MMS in the thread. When we are in the corrupted state, this has an incorrect value: document.getElementById('main-wrapper').scrollLeft // = 320 This shows well that we have a scroll request somewhere. Still investigating.
Found a fix, writing tests right now
Assignee: azasypkin → felash
Comment on attachment 8449467 [details] [review] github PR Looks and works good! Just left few nits and question at github.
Comment on attachment 8449467 [details] [review] github PR Ready :) I've added a 3rd commit with your follow-ups !
Comment on attachment 8449467 [details] [review] github PR (In reply to Julien Wajsberg [:julienw] from comment #11) > Comment on attachment 8449467 [details] [review] > github PR > > Ready :) > > I've added a 3rd commit with your follow-ups ! Great, r=me with the last tiny nit in unit test adressed! Thanks!
Attachment #8449467 - Flags: review?(azasypkin) → review+
Thanks ! squashed and rebased, waiting for a green-ish travis or try build
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.