mozilla-b2g/gaia — pull request #12454 by tafsiri (2013-09-25 20:45:56+00:00)
Title: Bug 919567. Stopwatch and Timer Panels do not update from underlying dat...

This commit is for https://bugzilla.mozilla.org/show_bug.cgi?id=919567. Given a stopWatch object the Stopwatch.Panel should set itself up accordingly, given a Timer object the Timer.panel should updated itself accordingly. While this is in support of https://bugzilla.mozilla.org/show_bug.cgi?id=910325 it does not add or modify any persistence related code (Stopwatch didn't have any, Timer did).

For Stopwatch/Stopwatch.Panel I've added this functionality and added some new tests in stopwatch_panel_test.js. I also had to change stopwatch_test.js a bit. Evelyn you may want to take a look.

Note: I assume that the stopwatch rolling back to 0 after 59:59 is in spec. If not we should file a bug

For Timer/Timer.Panel, I've added some tests in timer_panel_test.js. Changes to timer.js mainly came from Rick but I had to modify it a bit.

With regards to the timer I had discussed refactoring this code with Rick, to centralize the Panels state changing code (pulling it out of onvisibilitychange and the onclick handlers). As well as eliminate the REACTIVATING states; and combine CANCELLED AND INITIALIZED (c.f. Stopwatch.Panel in this patch for what I am thinking of). This commit doesn't have that refactoring (and does fix the bug), but I still think it would be a good idea. I find the code in onvisibilitychange a little more complex that it likely needs with the nested state checks.

You'll also notice that in the tests where I set timers directly, I'm not testing creating a cancelled timer, or a reactivating one. The cancelled state is one that is in between running and initialized, and the reactivating state also seems to be an intermediate one, these do not really have on screen representations of their own (i think of cancelled as the transition to initialized/reset/not running). So i'm not sure if these should be tested (though directly creating a cancelled timer would not show the timer creating dialog—but none of the code does this as we never persist cancelled timers).

Anyways, I think Rick felt that it was too big of a refactor to be worthwhile. But i think that as states get added (e.g. expired), it may be useful to un-nest some of that code.

Let me know if you have any thoughts. Thanks