1041432 - [dolphin][flame] dock bar position moves up when return to homescreen from E.ME

Status: RESOLVED WONTFIX

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

Description

[Peipei Cheng (needinfo if you need my action)]

Attachment: STR video

Steps to reproduce
----------------------------------------------------------------------------
1. Start device
2. Tap on "I'm thinking of...", keyboard will appear
3. Tap on Home Button
   --> You will see the dock bar was moved up. 

Both Flame and dolphin have this problem. But V1.3 does not have this issue.

Attached is a STR video.

Test build
---------------------------------------------------------------------------
Gaia      621d152f89347c79619aa909ad62cc2ac9d3ab5b
Gecko     https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/83b7be7fb33f
BuildID   20140720160206
Version   30.0
ro.build.version.incremental=109
ro.build.date=Mon Jun 16 16:51:29 CST 2014

Comment 1

[ben.song]

Attachment: the four footer buttons position incorrect.png

In this series of homescreen questions, there is shown that the four buttons of footer moved up a certain distance, through the analysis found, there is a reason for this: when the user opens the search page in homescreen, and open input method in it, then homescreen height would change and trigger resize event. However, due to the impact of lowmemorykiller, homescreen would be killed, and homescreen will be regenerated, but has four buttons below the screen to the center. Because the time during homescreen killed to reopened, its height haven't changed,so the position of the four button would be incorrect.
Modification method is that reset to the initial value of homescreen height when it reopened after suddenly killed.

Comment 2

[James Zhang (Spreadtrum)]

[Blocking Requested - why for this release]:

Comment 3

[James Zhang (Spreadtrum)]

Ben, please attach your patch here.

Comment 4

[ben.song]

Attachment: page_of_homescreen_overlap_and_footerbutton_of homescreen_moveup.patch

This patch deals with two problems:
1.When IME shows but homescreen been killed and restarted,four footer buttons of homescreen move up.(I havn't deal with that when we switch IME on homescreen,the button moveup temporarily,if you need to deal with it,i can do it.)
2.when monkey test several hours,to switch homescreen and apps and slide the screen in the process,pages of homescreen happen to overlap.

Comment 5

[Peipei Cheng (needinfo if you need my action)]

(In reply to ben.song from comment #4)
> Created attachment 8462336 [details] [diff] [review]
> page_of_homescreen_overlap_and_footerbutton_of homescreen_moveup.patch
> 
> This patch deals with two problems:
> 1.When IME shows but homescreen been killed and restarted,four footer
> buttons of homescreen move up.(I havn't deal with that when we switch IME on
> homescreen,the button moveup temporarily,if you need to deal with it,i can
> do it.)
> 2.when monkey test several hours,to switch homescreen and apps and slide the
> screen in the process,pages of homescreen happen to overlap.

Ben, what do you mean by "button move up"? If it mean the dock bar move up, I think we should fix it. Because from user's perspective, we don't expect the dock bar to move up.

Comment 6

[ben.song]

(In reply to pcheng from comment #5)
> (In reply to ben.song from comment #4)
> > Created attachment 8462336 [details] [diff] [review]
> > page_of_homescreen_overlap_and_footerbutton_of homescreen_moveup.patch
> > 
> > This patch deals with two problems:
> > 1.When IME shows but homescreen been killed and restarted,four footer
> > buttons of homescreen move up.(I havn't deal with that when we switch IME on
> > homescreen,the button moveup temporarily,if you need to deal with it,i can
> > do it.)
> > 2.when monkey test several hours,to switch homescreen and apps and slide the
> > screen in the process,pages of homescreen happen to overlap.
> 
> Ben, what do you mean by "button move up"? If it mean the dock bar move up,
> I think we should fix it. Because from user's perspective, we don't expect
> the dock bar to move up.

   Yes,I find the cause of the dock bar move up is that time differece of IME remove and screen height recover.I would fix it.

Comment 7

[ben.song]

(In reply to pcheng from comment #5)
> (In reply to ben.song from comment #4)
> > Created attachment 8462336 [details] [diff] [review]
> > page_of_homescreen_overlap_and_footerbutton_of homescreen_moveup.patch
> > 
> > This patch deals with two problems:
> > 1.When IME shows but homescreen been killed and restarted,four footer
> > buttons of homescreen move up.(I havn't deal with that when we switch IME on
> > homescreen,the button moveup temporarily,if you need to deal with it,i can
> > do it.)
> > 2.when monkey test several hours,to switch homescreen and apps and slide the
> > screen in the process,pages of homescreen happen to overlap.
> 
> Ben, what do you mean by "button move up"? If it mean the dock bar move up,
> I think we should fix it. Because from user's perspective, we don't expect
> the dock bar to move up.

   But the patch is the problem happened in the monkey test,it is diferent from above.So please review it.Thanks.

Comment 8

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Comment on attachment 8462336 [details] [diff] [review]
page_of_homescreen_overlap_and_footerbutton_of homescreen_moveup.patch

Hi Ben,

I don't see where the problem is. 
To me, it looks like the dock just has a little jitter when keyboard fadeout.

And since your patch had modified system, I think you should ask alive for review, not me.

Comment 9

[ben.song]

(In reply to Tzu-Lin Huang [:dwi2][:tzhuang] from comment #8)
> Comment on attachment 8462336 [details] [diff] [review]
> page_of_homescreen_overlap_and_footerbutton_of homescreen_moveup.patch
> 
> Hi Ben,
> 
> I don't see where the problem is. 
> To me, it looks like the dock just has a little jitter when keyboard fadeout.
> 
> And since your patch had modified system, I think you should ask alive for
> review, not me.

Hi Tzn-Lin,

 Ok,thanks.The bug occurs in the monkey test,mainly homescreen overlap when slide during the switch between homescreen and apps,and dock bar move up when homescreen been suddenly killed.

Comment 10

[ben.song]

Attachment: switch_between_IME_and_homescreen_dock_moveup_briefly.patch

Hi pcheng,

   This patch is for the bug you have described,when switch between IME and homescreen, the dock bar move up briefly,please review it.

   Thanks.

Comment 11

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8462336 [details] [diff] [review]
page_of_homescreen_overlap_and_footerbutton_of homescreen_moveup.patch

Review of attachment 8462336 [details] [diff] [review]:
-----------------------------------------------------------------

r- for hack keyboard height in appWindow.

Comment 12

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #11)
> Comment on attachment 8462336 [details] [diff] [review]
> page_of_homescreen_overlap_and_footerbutton_of homescreen_moveup.patch
> 
> Review of attachment 8462336 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r- for hack keyboard height in appWindow.

And I don't understand why we need to hack keyboard in mozbrowserloadend event handler.

Comment 13

[ben.song]

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #12)
> (In reply to Alive Kuo [:alive][NEEDINFO!] from comment #11)
> > Comment on attachment 8462336 [details] [diff] [review]
> > page_of_homescreen_overlap_and_footerbutton_of homescreen_moveup.patch
> > 
> > Review of attachment 8462336 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > r- for hack keyboard height in appWindow.
> 
> And I don't understand why we need to hack keyboard in mozbrowserloadend
> event handler.

Hi Alive,

   The patch to hack keyboard in mozbrowserloadend is for the problem when you opened IME in search of homescreen,and homescreen be killed in this moment,this dock bar would appear in the center of homescreen.So we should check the height of keyboard and invoke resize function,let the height recover origin value in the moment of a new homescreen render.The reason why I hold it in mozbrowserloadend is the event be at last of a new homescreen render and it could bring good experience to users.

Comment 14

[ben.song]

Hi pei,

   Why the attachment #8462975 [details] [diff] [review] is the state of review-?  thanks.

Comment 15

[Peipei Cheng (needinfo if you need my action)]

(In reply to ben.song from comment #14)
> Hi pei,
> 
>    Why the attachment #8462975 [details] [diff] [review] [diff] [review] is the state of
> review-?  thanks.

I'm not the right person to review your patch, thus review-

Comment 16

[ben.song]

(In reply to Peipei Cheng from comment #15)
> (In reply to ben.song from comment #14)
> > Hi pei,
> > 
> >    Why the attachment #8462975 [details] [diff] [review] [diff] [review] [diff] [review] is the state of
> > review-?  thanks.
> 
> I'm not the right person to review your patch, thus review-

Hi pei,

  OK,thanks.

Comment 17

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8462975 [details] [diff] [review]
switch_between_IME_and_homescreen_dock_moveup_briefly.patch

Review of attachment 8462975 [details] [diff] [review]:
-----------------------------------------------------------------

No idea why this needs my review.

Comment 18

[Peipei Cheng (needinfo if you need my action)]

ben.song, I think the patch for homescreen overlap issue should goes to Bug 1041423. For this one, we'd better only track the dock move up problem.

Comment 19

[Ivan Tsay (:ITsay)]

Hi Gregor,

Could you take a look this homescreen issue and see if you can find someone to help the review on the patch provided by our partner. Thank you.

Comment 20

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to ben.song from comment #13)
> (In reply to Alive Kuo [:alive][NEEDINFO!] from comment #12)
> > (In reply to Alive Kuo [:alive][NEEDINFO!] from comment #11)
> > > Comment on attachment 8462336 [details] [diff] [review]
> > > page_of_homescreen_overlap_and_footerbutton_of homescreen_moveup.patch
> > > 
> > > Review of attachment 8462336 [details] [diff] [review]:
> > > -----------------------------------------------------------------
> > > 
> > > r- for hack keyboard height in appWindow.
> > 
> > And I don't understand why we need to hack keyboard in mozbrowserloadend
> > event handler.
> 
> Hi Alive,
> 
>    The patch to hack keyboard in mozbrowserloadend is for the problem when
> you opened IME in search of homescreen,and homescreen be killed in this
> moment,this dock bar would appear in the center of homescreen.So we should
> check the height of keyboard and invoke resize function,let the height
> recover origin value in the moment of a new homescreen render.The reason why
> I hold it in mozbrowserloadend is the event be at last of a new homescreen
> render and it could bring good experience to users.

The patch is problematic in several ways,

If there is an app loading at background you will kill the foreground keyboard.
If there is an app loading at background you will force resizing and then reflow.
AppWindow should not touch KeyboardManager for any reason.

Comment 21

[ben.song]

(In reply to Peipei Cheng from comment #18)
> ben.song, I think the patch for homescreen overlap issue should goes to Bug
> 1041423. For this one, we'd better only track the dock move up problem.

OK,I would extract two patch from the attachment #8462336 [details] [diff] [review],and put them in their suited bug.

Comment 22

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8462975 [details] [diff] [review]
switch_between_IME_and_homescreen_dock_moveup_briefly.patch

Since this patch is about everything.me, I think I'm not the right person to review it, either.

Comment 23

[Luke Chang [:lchang] (inactive)]

Attachment: quick patch

I tried to make a quick patch for this bug.

Alive, would you mind giving me some feedback? Thanks.

Comment 24

[ben.song]

Attachment: keyboard_open_in_search_bar_and_homescreen_be_killed_dock_bar_moveup.patch

When keyboard opened in the search bar and at the moment homescreen be killed,dock bar would move up.The reason of it is when keyboard opened,the height of the page element of homescreen would reduce the height of keyboard,but when homescreen be killed,the height would not recover,so the dock bar would move up.For this,I add a checker in the process of restart of homescreen,in a event of mozbrowserloadend,and add a judge that the apps if or not be homescreen.if it is,I would recover the height of the homescreen.

Hi pei,

   Please add reviewers,thanks.

Comment 25

[James Zhang (Spreadtrum)]

Ben, please wait mozilla review.

Comment 26

[ben.song]

(In reply to James Zhang (Spreadtrum) from comment #25)
> Ben, please wait mozilla review.

Hi James,

   OK.

Comment 27

[Gregor Wagner [:gwagner]]

This seems a combination of app_window and Keyboard to me. Tim can you help with reviewers?
Not sure a homescreen reviewer is helpful but I can find one if needed. Cristian is out so it would be best to ping vingtetun.

Comment 28

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Comment on attachment 8462975 [details] [diff] [review]
switch_between_IME_and_homescreen_dock_moveup_briefly.patch

Hi Ben,

I am not review of either e.me or homescreen, so I cancel it.

However, I think Luke has gave a more proper solution for this bug on comment 23. I think we should elaborate more on that patch.

Comment 29

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8463322 [details] [review]
quick patch

See github

Comment 30

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I actually think this should be addressed in the home screen app -- Homescreen should wait for keyboard hidden before working on resizing itself etc.

Homescreen reviewers should followup with Ben from patch comment 24.

Comment 31

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I meant to say the patch in comment 10.

Comment 32

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Hi Wayne,

We need to know the priority of the bug. 
It is easy to reproduce on dolphin. 

I think it is no harm under normal circumstances but if homescreen is OOM killed at the moment it occurs, it would be like this https://bug1041432.bugzilla.mozilla.org/attachment.cgi?id=8459468

Comment 33

[Luke Chang [:lchang] (inactive)]

So far, I think there are two issue in this bug:

1. When hiding the keyboard from E.ME, you will see the dock move up to the middle of screen temporarily and soon move back to bottom shortly. (refer to comment 0)

2. The height of the homescreen window becomes incorrect if homescreen app is killed while using E.ME. (refer to comment 1)

Actually, these two issues are irrelevant. I can reproduce the second issue on unagi (v1.4) even though there isn't the fist issue on it. The first issue is just an UX issue and can be fixed through CSS.

I recommend separating this bug into two bugs to clarify these two issue.

Ben, how do you think?

Comment 34

[Wayne Chang [:wchang]]

(In reply to Luke Chang [:lchang] from comment #33)
> So far, I think there are two issue in this bug:
> 
> 1. When hiding the keyboard from E.ME, you will see the dock move up to the
> middle of screen temporarily and soon move back to bottom shortly. (refer to
> comment 0)
> 
> 2. The height of the homescreen window becomes incorrect if homescreen app
> is killed while using E.ME. (refer to comment 1)
> 
> Actually, these two issues are irrelevant. I can reproduce the second issue
> on unagi (v1.4) even though there isn't the fist issue on it. The first
> issue is just an UX issue and can be fixed through CSS.
> 
> I recommend separating this bug into two bugs to clarify these two issue.
> 
> Ben, how do you think?

1. isn't really a blocker since its only for a split second and have near zero user impact.

for 2. - as i've seen it demo'ed by Luke, the bar gets stuck half way on the screen and does look nasty when happens. I think we can take a low risk/effort fix on it. It can be recovered by launching an app and exiting it.

Luke, I am 1.4-'ing this bug here, if you have time for a fix on 2 (using this bug or a new bug), please nom and NI me to request for 1.4+.

Comment 35

[ben.song]

(In reply to Luke Chang [:lchang] from comment #33)
> So far, I think there are two issue in this bug:
> 
> 1. When hiding the keyboard from E.ME, you will see the dock move up to the
> middle of screen temporarily and soon move back to bottom shortly. (refer to
> comment 0)
> 
> 2. The height of the homescreen window becomes incorrect if homescreen app
> is killed while using E.ME. (refer to comment 1)
> 
> Actually, these two issues are irrelevant. I can reproduce the second issue
> on unagi (v1.4) even though there isn't the fist issue on it. The first
> issue is just an UX issue and can be fixed through CSS.
> 
> I recommend separating this bug into two bugs to clarify these two issue.
> 
> Ben, how do you think?
Hi Luke,

   You are right,I would set up a new bug for 2.But 1 is also important for pcheng has emphasised.OK?
   Thanks.

Comment 36

[Luke Chang [:lchang] (inactive)]

Hi Wayne,

Thanks. According to comment 35, Ben will file another bug for the second issue so let's track there.


Hi Ben,

For the first issue, you may refer to https://github.com/luke-chang/gaia/commit/4f8ccce947081cc110c8ddb35b10db7282de1045#diff-0 ("homescreen.css" part) to fix it.

Comment 37

[Peipei Cheng (needinfo if you need my action)]

(In reply to Wayne Chang [:wchang] from comment #34)
> (In reply to Luke Chang [:lchang] from comment #33)
> > So far, I think there are two issue in this bug:
> > 
> > 1. When hiding the keyboard from E.ME, you will see the dock move up to the
> > middle of screen temporarily and soon move back to bottom shortly. (refer to
> > comment 0)
> > 
> > 2. The height of the homescreen window becomes incorrect if homescreen app
> > is killed while using E.ME. (refer to comment 1)
> > 
> > Actually, these two issues are irrelevant. I can reproduce the second issue
> > on unagi (v1.4) even though there isn't the fist issue on it. The first
> > issue is just an UX issue and can be fixed through CSS.
> > 
> > I recommend separating this bug into two bugs to clarify these two issue.
> > 
> > Ben, how do you think?
> 
> 1. isn't really a blocker since its only for a split second and have near
> zero user impact.

Wayne, I think 1 is still a block, because:
1. This issue is not reproducible on V1.3 and V1.3t. So it's a regression.
2. I think E.Me is a frequently use feature. User will do such switch actions a lot. So they will see this problem a lot.

Comment 38

[Danny Liang [:dliang]]

(In reply to Luke Chang [:lchang] from comment #23)
> Created attachment 8463322 [details] [review]
> quick patch
> 
> I tried to make a quick patch for this bug.
> 
> Alive, would you mind giving me some feedback? Thanks.

w/o the patch, we can reproduce this issue in monkey test.
Applying the patch, this issue is gone in 60hrs monkey test and there is no crash/hang/b2g kill.

Comment 39

[Kevin Grandon :kgrandon]

Mass update: Resolve wontfix all issues with legacy homescreens.

As of 2.6 we have a new homescreen and having these issues open is confusing. All issues will block bug 1231115 so we can use that to re-visit any of these if needed.