Closed
Bug 814840
Opened 12 years ago
Closed 12 years ago
Remove workaround about 'hold-home' during FTU
Categories
(Firefox OS Graveyard :: Gaia::First Time Experience, defect, P1)
Firefox OS Graveyard
Gaia::First Time Experience
ARM
Gonk (Firefox OS)
Tracking
(blocking-b2g:leo+, blocking-basecamp:-, b2g18+ fixed, b2g-v1.1hd fixed)
VERIFIED
FIXED
People
(Reporter: borjasalguero, Assigned: alive)
References
Details
Attachments
(3 files)
As we agree through IRC, It would be necessary to 'refactor' this workaround in order to align the code with 'System App' structure. It's a refactor, so once the patch would be ready the code should be align with the rules of 'System App' and all the functionality should be working:
- Timing during FTU should be right
- Hold home during FTU should not be working
Reporter | ||
Updated•12 years ago
|
blocking-basecamp: --- → ?
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → ARM
Updated•12 years ago
|
Assignee: nobody → fernando.campo
Updated•12 years ago
|
blocking-basecamp: ? → +
Priority: -- → P1
Comment 1•12 years ago
|
||
Thanks for filing the bug and work on this.
Updated•12 years ago
|
Assignee: fernando.campo → nobody
Comment 2•12 years ago
|
||
(In reply to Borja Salguero [:borjasalguero] from comment #0)
> As we agree through IRC, It would be necessary to 'refactor' this workaround
> in order to align the code with 'System App' structure. It's a refactor, so
> once the patch would be ready the code should be align with the rules of
> 'System App' and all the functionality should be working:
> - Timing during FTU should be right
> - Hold home during FTU should not be working
What is the downside of letting the workaround in for v1?
Flags: needinfo?(fbsc)
Flags: needinfo?(francisco.jordano)
Comment 3•12 years ago
|
||
I'm renominating this bug and let's take a blocking decision on it while it is clear what are the side effect of not removing the workaround.
blocking-basecamp: + → ?
Flags: needinfo?(timdream+bugs)
Reporter | ||
Comment 4•12 years ago
|
||
Currently the workaround is working as expected. However, as a 'workaround', someday should be removed in order to preserve the 'structure' that [:timdream] mention in his mail about 'System'.
On the other hand it's true that we have a lot of bb+ to focus in, so I would consider it as a polish issue (bb-).
Flags: needinfo?(fbsc)
Comment 5•12 years ago
|
||
Sounds good. As much as I would like to remove workaround it does not make sense to block the release for that.
Flags: needinfo?(timdream+bugs)
Flags: needinfo?(francisco.jordano)
Comment 6•12 years ago
|
||
Borjasalguero and Vivien,
I am no code sanity police; bb- is fine.
Just remember that if there are more unforeseen (blocking+!) bug found in the current workaround FTU launching code, it might be saner to implement this bug for v1. I hope that doesn't happen :)
Updated•12 years ago
|
Summary: [FTU] [Follow-up] Remove workaround about 'hold-home' during FTU → Remove workaround about 'hold-home' during FTU
Updated•12 years ago
|
blocking-basecamp: ? → -
Comment 7•12 years ago
|
||
Alive, are you interesting taking this non-v1, master-only, non-trivial bug?
Vivien, I would really like to fix this, even if that means diverge master/v1-train.
Flags: needinfo?(alive)
Flags: needinfo?(21)
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → alive
Assignee | ||
Comment 8•12 years ago
|
||
Being glad to remove work around,
Diverge between master and v1-train is unavoidable IMO.
Flags: needinfo?(alive)
Assignee | ||
Comment 9•12 years ago
|
||
WIP patch v1:
https://github.com/alivedise/gaia/tree/bugzilla/814840/remove-ftu-workaround
https://github.com/alivedise/gaia/commit/cb9312ddb21badc551040f807cccd4e7d734d8af
* Done
1. Move all ftu variable/function out of WM.
2. Fix related function call in system app.
* Todo
1. Sanity lockscreen unlock function
2. Variable rename under ftu launcher
3. Utilize app window object
4. Unit test
5. Fix ftu close issue
Comment 10•12 years ago
|
||
Diverging is fine too imo. It would make our life harder for merging other patches but this does not seems a god strategy to be blocked by old code dirtyness.
Flags: needinfo?(21)
Assignee | ||
Comment 11•12 years ago
|
||
Status update:
Nearly done, but I am blocked by quick screen timeout when FTU running :/
Comment 12•12 years ago
|
||
This bug is blocking a tef+ bug 845251 so nominating it for tef+ too
blocking-b2g: --- → tef?
Comment 14•12 years ago
|
||
When verifying this bug please check STR from bug 832895 too.
Keywords: qawanted
Looks like we need the patch, please mark as needinfo :nhirata so that I'm notified to check it as soon as the patch lands.
Updated•12 years ago
|
Flags: needinfo?(nhirata.bugzilla)
Assignee | ||
Comment 16•12 years ago
|
||
(In reply to Maria Angeles Oteo:oteo from comment #12)
> This bug is blocking a tef+ bug 845251 so nominating it for tef+ too
Bug 845251 is not tef+, do you link the wrong bug #?
Bug 832895 is terrible because it's power consuming forever and the screen won't go off anyway before FTU ends.
Assignee | ||
Updated•12 years ago
|
Flags: needinfo?(oteo)
Assignee | ||
Comment 17•12 years ago
|
||
Patch v1.
No unit test at this point.
I prefer to create one in another bug because it seems somewhat complex.
Attachment #721054 -
Flags: review?(timdream)
Comment 18•12 years ago
|
||
Comment on attachment 721054 [details]
https://github.com/mozilla-b2g/gaia/pull/8435
r=me, more manual tests is needed (discussed offline) to ensure this will not break anything.
Attachment #721054 -
Flags: review?(timdream) → review+
Comment 19•12 years ago
|
||
(In reply to Alive Kuo [:alive] (Life is a struggle.) from comment #16)
> (In reply to Maria Angeles Oteo:oteo from comment #12)
> > This bug is blocking a tef+ bug 845251 so nominating it for tef+ too
>
> Bug 845251 is not tef+, do you link the wrong bug #?
You are right, Alive, I made a mistake with the number ;)
The tef+ bug is Bug 837558 that according to its comments it seems that bug 814840 turns out to be its root cause.
Flags: needinfo?(oteo)
Assignee | ||
Comment 20•12 years ago
|
||
Great. Sounds like we need more testing!
(In reply to Maria Angeles Oteo:oteo from comment #19)
> (In reply to Alive Kuo [:alive] (Life is a struggle.) from comment #16)
> > (In reply to Maria Angeles Oteo:oteo from comment #12)
> > > This bug is blocking a tef+ bug 845251 so nominating it for tef+ too
> >
> > Bug 845251 is not tef+, do you link the wrong bug #?
>
> You are right, Alive, I made a mistake with the number ;)
>
> The tef+ bug is Bug 837558 that according to its comments it seems that bug
> 814840 turns out to be its root cause.
Assignee | ||
Comment 22•12 years ago
|
||
1. make NOFTU=1 doesn't work for me even on master.
2. QA would help on gaia uitest stuff about Window Manager change. (Talked with atsai)
I will keep testing and then merge to master tomorrow if nothing bad happens.
Updated•12 years ago
|
QA Contact: nhirata.bugzilla
Assignee | ||
Comment 23•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 24•12 years ago
|
||
(In reply to Alive Kuo [:alive] (Life is a struggle.) from comment #23)
> https://github.com/mozilla-b2g/gaia/commit/
> b4db4ccac38811b1c1992cba7781049cc90ce1f6 master
Thanks for the effort!!
Updated•12 years ago
|
Flags: needinfo?(nhirata.bugzilla)
Reporter | ||
Comment 25•12 years ago
|
||
This patch create a regression in FTU when PIN CODE it's required. Please fix the following bug https://bugzilla.mozilla.org/show_bug.cgi?id=849200
While testing this bug, I found : https://bugzilla.mozilla.org/show_bug.cgi?id=849443
Also to note that the PUK has the same type of issue as bug 849200, I see the PUK appear in 2 different screens in the FTU if I don't bother to complete it.
thanks Borja for pointing out the SIM lock/Pin Code.
Flags: needinfo?(nhirata.bugzilla)
Marking as verified:
"mozilla-central" revision="cb432984d5ce"
"integration/gaia-central" revision="df56729d46b7"
gecko.git revision="0be8fda9a032227e46bb011fda43aa589c632033"
gaia.git revision="c7ac21c48e3718a178340a56b17a0508ae903aba"
2013-03-08-03-10-58_MC
Status: RESOLVED → VERIFIED
Updated•12 years ago
|
blocking-b2g: tef? → ---
Assignee | ||
Comment 28•12 years ago
|
||
We need to decide this is tef? because the patch to bug 837558(tef+) depends on this bug.
blocking-b2g: --- → tef?
Comment 29•12 years ago
|
||
This patch is pretty big so I'm definitely very concerned with a possible regression if we pick this up for v1.0.1.
Does comment 26 of bug 837558 not point to a way that we can pull in that fix without this bug landing on v1.0.1?
Assignee | ||
Comment 30•12 years ago
|
||
(In reply to Michael Vines [:m1] [:evilmachines] from comment #29)
> This patch is pretty big so I'm definitely very concerned with a possible
> regression if we pick this up for v1.0.1.
>
> Does comment 26 of bug 837558 not point to a way that we can pull in that
> fix without this bug landing on v1.0.1?
OK I see. I am fine ;) I just want to know why tef? is set to --- without any reason.
blocking-b2g: tef? → ---
Comment 31•12 years ago
|
||
Yes, I'm sorry for neglecting to add a comment and thanks for challenging. It's so easy to clear a flag by accident!
Assignee | ||
Comment 32•12 years ago
|
||
Great! and NVM, thanks for clarification.
Comment 34•12 years ago
|
||
Can we consider uplifting this patch to v1-train? This patch will fix Bug 864620 issue. I have set leo? flag to this bug.
Should the status of this bug also need to change? (It's verified fixed now.)
blocking-b2g: --- → leo?
Assignee | ||
Comment 35•12 years ago
|
||
(In reply to leo.bugzilla.gaia from comment #34)
> Can we consider uplifting this patch to v1-train? This patch will fix Bug
> 864620 issue. I have set leo? flag to this bug.
> Should the status of this bug also need to change? (It's verified fixed now.)
No, uplifting process/leo? triage/bug status are going independently.
Comment 36•12 years ago
|
||
(In reply to leo.bugzilla.gaia from comment #34)
> Can we consider uplifting this patch to v1-train? This patch will fix Bug
> 864620 issue. I have set leo? flag to this bug.
> Should the status of this bug also need to change? (It's verified fixed now.)
We can ask the engineers to nominate for approval with a risk/reward evaluation, but this is not a blocker.
blocking-b2g: leo? → -
tracking-b2g18:
--- → +
Comment 37•11 years ago
|
||
FYI, the original FTU launching code was implemented in bug 803101.
Comment 38•11 years ago
|
||
We have seem many, many regression on v1-train for the last few months and it's worthy to re-think whether or not we should take this patch to v1-train.
Alive told me yesterday even now, the likelihood of uplifting this bug and break things is less than more potential regressions on v1-train. We would also have to backout many of the hacks put in place in v1-train that supposedly to replace this fix.
So triage, should we do it? I believe Alive have more to add on risk/reward evaluation.
blocking-b2g: - → leo?
Flags: needinfo?(alive)
Assignee | ||
Comment 39•11 years ago
|
||
I believe the risk(bugs regressed after uplifting) is less then keep regression grow up, because during these three months all FTU relevant bugs on v1-train doesn't occur to master.
All bugs I could find now:
Bug 854842 - blank screen when use customize power on animation on v1-train & v1.0.1
Bug 837558 - [FTU] Sim Card Toolkits may interfere with FTU process
Bug 875646 - [Buri][FTU] boot-up cannot complete to enter home screen
Bug 831697 - White screen after receive a call in FTE
(The following two are regressions caused by a workaround patch of bug 831697)
Bug 877062 - [v1-train][FTU] You can kill FTU by long press the home key again
Bug 876340 - [Sound] Vibrate option is not available during/after FTU
Flags: needinfo?(alive)
Comment 40•11 years ago
|
||
(In reply to Alive Kuo [:alive] from comment #39)
Let's have this uplifted to v1-train!
Thanks
blocking-b2g: leo? → leo+
Comment 41•11 years ago
|
||
I was not able to uplift this bug to v1-train. If this bug has dependencies which are not marked in this bug, please comment on this bug. If this bug depends on patches that aren't approved for v1-train, we need to re-evaluate the approval. Otherwise, if this is just a merge conflict, you might be able to resolve it with:
git checkout v1-train
git cherry-pick -x -m1 b4db4ccac38811b1c1992cba7781049cc90ce1f6
<RESOLVE MERGE CONFLICTS>
git commit
Flags: needinfo?(alive)
Assignee | ||
Comment 42•11 years ago
|
||
https://github.com/mozilla-b2g/gaia/commit/84b027e1deff1015a69611b3731b190731cdb391
Uplift FTU launcher to v1-train,
Backout bug 854842, bug 875646, bug 831697, bug 877062.
status-b2g18:
--- → fixed
Flags: needinfo?(alive)
Comment 43•11 years ago
|
||
v1.1.0hd: 84b027e1deff1015a69611b3731b190731cdb391
v1.1.0hd: 950f81700da59c06e6cd205e415fc9457936f567
status-b2g-v1.1hd:
--- → fixed
Comment 44•11 years ago
|
||
Hi Alive,
I have seen the following commit in v1-train.
* Revert "Bug 854842 - Check FTU running before visibility change"
This reverts commit ee55b12d8bf8fcfc49f271e7f4a1086f47eeea96.
Because we are uplifting bug 814840 to v1-train.
And somehow this revert causes the following issue.
1. Launch CutTheRope game and observe it plays sounds.
2. Press Power button to screen-off.
3. The sound still plays. <== issue.
4. Press power button and unlock the lockscreen. Now the sounds stops. <== weird.
And If I put the following reverted code back in, the issue goes away.
the https://github.com/mozilla-b2g/gaia/pull/9038/files
Should we put the reverted code back in?
Flags: needinfo?(alive)
Assignee | ||
Comment 45•11 years ago
|
||
It's behavior from bug 828283, not by reverting 854842.
(I remember we had discussed 828283 somewhere)
Flags: needinfo?(alive)
Comment 46•11 years ago
|
||
(In reply to Alive Kuo [:alive] from comment #45)
> It's behavior from bug 828283, not by reverting 854842.
> (I remember we had discussed 828283 somewhere)
How about #4 below? The sound stops when lockscreen unlocks. Should I create a new bug for this?
1. Launch CutTheRope game and observe it plays sounds.
2. Press Power button to screen-off.
3. The sound still plays. <== issue.
4. Press power button and unlock the lockscreen. Now the sounds stops. <== weird.
Thanks
Flags: needinfo?(alive)
Assignee | ||
Comment 47•11 years ago
|
||
Hi,
I am unlucky to find cut-the-rope app so I just add an audio tag to test app,
and I cannot reproduce what you mentioned. The sounds still plays after lockscreen unlocks.
Any further info?
Flags: needinfo?(alive)
Comment 48•11 years ago
|
||
(In reply to Alive Kuo [:alive] from comment #47)
Let me upload the cuttherope application.zip and a repro video. Let me know if this helps.
I tried imdb.com with browser app and it doesn't have this issue. (A movie trailer video still plays/makes sounds on lockscreen after screen-off/on)
By the way, my college already created a new bug for #3. You can close that one if the #3 behavior is expected. And let me know if #4 is an issue, I will create a new bug.
Assignee | ||
Comment 49•11 years ago
|
||
(In reply to hanj.kim25 from comment #48)
> Created attachment 773737 [details]
> cuttherope_application.zip
>
> (In reply to Alive Kuo [:alive] from comment #47)
>
> Let me upload the cuttherope application.zip and a repro video. Let me know
> if this helps.
>
> I tried imdb.com with browser app and it doesn't have this issue. (A movie
> trailer video still plays/makes sounds on lockscreen after screen-off/on)
>
>
> By the way, my college already created a new bug for #3. You can close that
> one if the #3 behavior is expected. And let me know if #4 is an issue, I
> will create a new bug.
I haven't tried your attachment but please do that because this thread is spamming some people. thanks.
Comment 50•11 years ago
|
||
(In reply to Alive Kuo [:alive] from comment #47)
Here's the repro video. Please listen to the sound of the video.
00:19 Sound stops at lockscreen.
00:27 Sound resume once page is moved. (as per https://bugzilla.mozilla.org/show_bug.cgi?id=891715#c2)
The new bug that my college created is 891715.
Attachment #773738 -
Flags: feedback?(alive)
Assignee | ||
Comment 51•11 years ago
|
||
Comment on attachment 773738 [details]
cuttherope_sound_issue_repro_video
I think this is a cut-the-rope specific issue since I am using the commit: 46ed9a47c75c6f8e9be8114ac7a819cc1efdc280 which is just before this bug uplifted to v1-train, it still happens(unlock causing the sound stops). I still don't know why but please don't discuss that on this bug.
Attachment #773738 -
Flags: feedback?(alive) → feedback+
Assignee | ||
Comment 52•11 years ago
|
||
Comment 53•11 years ago
|
||
v1.1.0hd: 2ec2c37dddf39e9ac54a56f8ae5c400d28fd4310
v1.1.0hd: 22d7b30bc88a6407e5b5cf0b1ea2ccaa394c6320
You need to log in
before you can comment on or make changes to this bug.
Description
•