Closed Bug 1038806 Opened 6 years ago Closed 2 years ago

Avoid calling `init` panel functions twice in Contacts


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

Not set


(Not tracked)



(Reporter: sergi, Unassigned)



(1 file)

`initSettings`, `initDetails` and `initForm` could all be called twice (for example by quickly tapping on the settings button inside Contacts) and then the callback would be executed twice, with unexpected consequences.
Assignee: nobody → sergi.mansilla
Attached file Github PR
This patch makes the three init functions (initDetails, initForm and initSettings) self-contained, without the need of external state variables, and avoids the possibility of loading dependencies twice or, more importantly, executing callbacks twice.
Attachment #8456275 - Flags: review?(francisco)
Target Milestone: --- → 2.0 S6 (18july)
Comment on attachment 8456275 [details] [review]
Github PR

Just a couple of nit on the github pr.

Could we add unit test to check that we call the functions just once?

Attachment #8456275 - Flags: review?(francisco)
Comment on attachment 8456275 [details] [review]
Github PR

Corrected nits and added tests that check that the callback is called twice.
Attachment #8456275 - Flags: review?(francisco)
About testing for the function to be run just once, since parts in the contacts test use these methods, it is not possible to test if it is run once, since it has been memoized already. I accept suggestions on how to test it. The only way I see right now is to make an entire new suit only for these cases. Is that acceptable?
Comment on attachment 8456275 [details] [review]
Github PR

Forwarding the review to Jose to speed up the process.
Attachment #8456275 - Flags: review?(francisco) → review?(jmcf)
Attachment #8456275 - Flags: review?(jmcf)
As per conversation with Sergi we are going to rely on a simpler solution that does not involve functions that return functions
Target Milestone: 2.0 S6 (18july) → ---
Assignee: sergi.mansilla → nobody
Firefox OS is not being worked on
Closed: 2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.