1089505 - [NFC][Software Home Button] yes/no button covered by SW home button

Status: VERIFIED FIXED

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

Description

[Walter Chen[:ypwalter][:wachen]]

*** Build Information
Gaia-Rev        1e48e3e40e0780c0cd07a3457e5fe2efeeb542d1
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/09fb60a37850
Build-ID        20141023001201
Version         34.0
Device-Name     flame
Base image: ***Please update this field to say if you're running the v180 or the v188 base image***

*** Description
yes/no button covered by SW home button

*** Steps to Reproduce
1. enable a software home button in settings
2. enable NFC in settings app
3. pair a first time connect NFC earphone with the phone

*** Expected Results
I can see yes/no or confirm/cancel button

*** Actual Results
I can't see yes/no or confirm/cancel button

*** Other Notes

*** Reproduction Frequency: 100%

Comment 1

[Tony Chung [:tchung]]

[Blocking Requested - why for this release]:

moving to SysFE team, who owns Soft Home button

Comment 2

[Manuel Casas Barrado [:mancas]]

Hey etienne, I've a question about how we want to solve this kind of situation. AFAIK, some dialogs (Sim pin and modal dialogs) recalculates the parent's height based on the layoutManager. This is good. However, the other ones, don't perform this operation. So the two possible options are:

1- Use window.layoutManager.height in every dialog, inside |dialog-overlay| element
2- Add this style to |dialog-overlay| element -> |height: calc(100% - var(--software-home-button-height) - var(--statusbar-height));|

The second option seems to be easier to fix all dialogs inside the parent element but does not take in account the keyboard height, per example.

What do you think?

Comment 3

[Michael Henretty [:mikehenrty][:mhenretty]]

Option 1 should only apply to dialogs that live either inside the app window, or inside the dialog overlay container. These windows/divs get resized according to the app that is in the foreground, and therefore the layout manager height calculation applies. If they are direct children (or siblings) of #screen, we don't want to use the layout manager. I haven't looked at the NFC dialog, so I am not sure.

If we go with option 2, we need to make sure this dialog still works with fullscreen apps (or make sure this dialog never appears over fullscreen apps).

Comment 4

[Manuel Casas Barrado [:mancas]]

Attachment: Proposed patch

Comment 5

[Manuel Casas Barrado [:mancas]]

Attachment: nfc-dialog.png

Comment 6

[Manuel Casas Barrado [:mancas]]

Attachment: nfc-dialog-full-screen-app.png

Now the dialog calculates the height based on the foreground app.

Comment 7

[Etienne Segonzac (:etienne)]

(In reply to Manuel Casas Barrado [:mancas] from comment #2)
> Hey etienne, I've a question about how we want to solve this kind of
> situation. AFAIK, some dialogs (Sim pin and modal dialogs) recalculates the
> parent's height based on the layoutManager. This is good. However, the other
> ones, don't perform this operation. So the two possible options are:
> 
> 1- Use window.layoutManager.height in every dialog, inside |dialog-overlay|
> element
> 2- Add this style to |dialog-overlay| element -> |height: calc(100% -
> var(--software-home-button-height) - var(--statusbar-height));|
> 
> The second option seems to be easier to fix all dialogs inside the parent
> element but does not take in account the keyboard height, per example.
> 
> What do you think?

Basically what Mike says :)
We're trying to move everything we can inside appWindows, and we should end with just 2-3 real system dialogs.
In the meantime, I think it's safe to go with the simpler solution.

Comment 8

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

Comment on attachment 8518855 [details]
nfc-dialog-full-screen-app.png

Add an unit test please and then we are ok to go.

Comment 9

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

Comment on attachment 8518853 [details] [review]
Proposed patch

comment 8

Comment 10

[Manuel Casas Barrado [:mancas]]

Comment on attachment 8518853 [details] [review]
Proposed patch

I've added the test cases for the patch. Please check it and let me know if we have to change something

Thanks!

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/25e4285f4c67198a3c3b20b18166bfd8862b6f4d

Please request Gaia v2.1 approval on this when you get a chance.

Comment 12

[Brogan Zumwalt [Inactive]]

Issue verified fixed on Flame 2.2

Actual Results: Confirmation buttons appear without being cut off by SHB

Device: Flame 2.2 Master (319mb)(Kitkat Base)(Shallow Flash)
BuildID: 20141113040205
Gaia: be8b0151d2f9a4c41fc63952128e0b723cd1161d
Gecko: ab137ddd3746
Version: 36.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Comment 13

[Brogan Zumwalt [Inactive]]

Leaving verify me for 2.1 uplift

Comment 14

[Manuel Casas Barrado [:mancas]]

Attachment: Patch v2.1

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): No regression AFAIK
[User impact] if declined: If the device has software button enabled, the user will be unable to click the buttons when trying to connect to another device.
[Testing completed]: Manually and unit tests
[Risk to taking this patch] (and alternatives if risky): Low
[String changes made]: No

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

v2.1: https://github.com/mozilla-b2g/gaia/commit/1fe5506ed8f7d176ab99136e049223b9ac87710f

Comment 16

[Elie]

Attachment: VIDEO0063[1].mp4

This issue has been successfully verified on Flame 2.1:
Gaia-Rev        f8d3bf44029e0afc0124600a4bb34dba8fc1ad21
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/f70a67a7f846
Build-ID        20141120001207
Version         34.0
Device-Name     flame
FW-Release      4.4.2

Comment 17

[Josh Cheng [:josh]]

(In reply to Elie from comment #16)
> Created attachment 8526477 [details]
> VIDEO0063[1].mp4
> 
> This issue has been successfully verified on Flame 2.1:
> Gaia-Rev        f8d3bf44029e0afc0124600a4bb34dba8fc1ad21
> Gecko-Rev      
> https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/f70a67a7f846
> Build-ID        20141120001207
> Version         34.0
> Device-Name     flame
> FW-Release      4.4.2

Hi Elie,
I think the video you attached is not correct. 
I do not see Software Home button in your video.
Please verify again and attach correct video.

Comment 18

[Yeojin Chung [:YeojinC]]

This issue is verified fixed on Flame 2.1.

Result: The confirmation dialog is displayed properly with the SHB enabled.

Device: Flame 2.1 (319mb, KK, Shallow Flash)
BuildID: 20141121001202
Gaia: 6c739275e963465658c18c7a9ebaa48cbe927d34
Gecko: 9bfc7a166a94
Version: 34.0 (2.1) 
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 19

[Yeojin Chung [:YeojinC]]

Attachment: Verified2.1.png