957577 - [Contacts] navigation.js should support several concurrent instances of navigationStack

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Contacts, defect)

Description

[Germán Toro del Valle (:gtorodelvalle)]

Currently the Contacts app creates 2 instances of the navigationStack object, one to deal with the navigation at the (so to call it) main Contacts app level and another one to deal with the navigation at the Contacts app Settings level. Due to how the z-index property is being set right now, there are cases when the navigation fails since it may end up setting a wrong z-index to the next view to show.

The navigationStack objects should share a common z-index value so that future navigations increase and decrease this common value properly.

It is important also to note that successive navigationStack instances sit on top of previous ones and consequently all its views have to be "closed" before interacting (go, back, home) with the lower one.

Comment 1

[Germán Toro del Valle (:gtorodelvalle)]

Attachment: 15107.html

At the end I chose the option which included less changes to avoid possible regressions.

I have confirmed that Ashay's patch to bug 956219 still works when pulling my patch on top of his so, Ashay, you may safely remove any changes to navigation.js from your pull request.

Comment 2

[Jose Manuel Cantera]

Comment on attachment 8357128 [details]
15107.html

thanks Germán

Comment 3

[Francisco Jordano [:arcturus] [:francisco]]

IMHO we should not add more time to the navigation object, specially taking into account that will be (hopefully) replaced, or totally delete when Hayda comes to an end.

I know that we still need to work, but navigation.js is not the part that I feel more confortable with, and if we can get rid of it ASAP, the better :)

Comment 4

[Jose Manuel Cantera]

We are only tweaking it in order to support multiple contact deletion. I agree that we should minify the changes on that component.

Comment 5

[Germán Toro del Valle (:gtorodelvalle)]

Yeah, totally agreed with all you said! :-)

Merged in master: https://github.com/mozilla-b2g/gaia/commit/d3fdcc2d25ccd6a0ac4ca1c5266898f7e33027bd

Comment 6

[ashayb2g]

@gtorodelvalle I tried this code and found some issue when in multiple delete screen if you go to search and then come back and do this process for some time you will find that screens are not in proper sequence, infact at times you'll see some altogether different screen. 

Please check the issue and kindly Reopen it.

Comment 7

[Germán Toro del Valle (:gtorodelvalle)]

Hi Ashay, nice catch! ;-) In fact, you just found a bug which is not really related to the code included in this patch or in yours regarding the multiple contact deletion. It was already there but I guess nobody tried it so far :-O

The problem is due to the fact that search.js was not considering the 2 possible navigationStack instances which may be controlling the application in the Contacts app. It was using the "main Contacts app navigationStack instance" by default (see https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/contacts/js/views/search.js#L123 and https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/contacts/js/views/search.js#L245).

I just created bug 958012 to deal with this issue and I'll include it as blocking to this very bug although it is not really a blocking issue but a related one to have everything working.