1058675 - [SHB] System Custom Dialogs have their buttons overlaid by the SHB

Status: VERIFIED FIXED

Description

[:Eli Perelman]

+++ This bug was initially created as a clone of Bug #1054716 +++

From Tony Chung:

i see the clipped buttons on the Install update screen.

See screenshot.

1) install 8/25 2.1 nightly build on flame

Gaia      e424c85eda87a40c0fa64d6a779c3fa368bf770b
Gecko     https://hg.mozilla.org/mozilla-central/rev/daa84204a11a
BuildID   20140825040204
Version   34.0a1
ro.build.version.incremental=110
ro.build.date=Fri Jun 27 15:57:58 CST 2014
B1TC00011230

2) enable soft home button in settings
3) settings > Device info> check for updates
4) find an update, and apply it via the status bar
5) when update is completed downloading, verify the install update screen is covered.

Expected:
- buttons are not covered

ActuaL:
- buttons covered

Comment 1

[:Eli Perelman]

Created attachment 8479120 [details]
b2g_homescreen_softHome_updateScreen.png

Comment 2

[:Eli Perelman]

Created attachment 8479181 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/23324

The patch for bug 1048593 should have resolved button clipping on dialogs when displayed with the SHB, but custom dialogs didn't inherit this fix because they were being appended to the body and not the #screen element. This patch changes custom dialogs to be appended to the #screen element, thereby inheriting the correct styles.

Comment 3

[Etienne Segonzac (:etienne)]

Comment on attachment 8479181 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/23324

This shared lib is used by a bunch of apps (camera, dialer...) so I'm not comfortable with adding system-only stuffs to it.

Also we have z-index-levels for dialogs already, we should not piggy back on the system-notification-banner level. (and maybe we don't need any change here since there's no transitions on those dialogs)

Is there something preventing us from target #dialog-screen with a system css rule?

Comment 4

[:Eli Perelman]

Created attachment 8479914 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/23375

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/9b6a855f052e5a425aedf4c526b7024a41ee28cf

Comment 9

[:Eli Perelman]

Comment on attachment 8479914 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/23375

Requesting uplift as the UI will be unusable for users of these dialogs if using the software home button. Functional polish.

[Bug caused by] (feature/regressing bug #): bug 1036339
[User impact] if declined: dialogs unusable with software home button enabled
[Testing completed]: yes, manual and automated
[Risk to taking this patch] (and alternatives if risky): low, should only impact system-level "custom" dialogs
[String changes made]: n/a

Comment 10

[Ryan VanderMeulen [:RyanVM]]

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

Comment 11

[Yeojin Chung [:YeojinC]]

This issue is verified fixed on Flame 2.1 & Flame 2.2:

Flame 2.1 (full flash)

Environmental Variables:
Device: Flame 2.1
BuildID: 20141008000201
Gaia: d71f8804d7229f4b354259d5d8543c25b4796064
Gecko: 7fa82c9acdf2
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Flame 2.2 (full flash)

Environmental Variables:
Device: Flame 2.2 Master
BuildID: 20141007040205
Gaia: 83de447d9ae9a59459d7a445f9348a254c661850
Gecko: 9ee9e193fc48
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

The dialog buttons are NOT overlaid by SHB, and everything on the screen is displayed properly.

Comment 12

[[e-mail], [e-mail]]

I don't join the bug before. But the patch makes regression about no device name while confirming Bluetooth file transfer.(bug 1082935)

The device name which we get from a async call(https://github.com/mozilla-b2g/gaia/pull/23375/files#diff-171571dcd7d2b8608173bb1b312b6e4eL184). The modification patch assigned an empty 'deviceName' before we get paired device from async all.

Comment 13

[:Eli Perelman]

Ian, you are correct, this would have caused a problem, but it was resolved again [1] through bug 1043556 which would put device name back in the correct position.

[1] https://github.com/mozilla-b2g/gaia/commit/4f24d93fdd8374347d96af07e75cd3e50705fbb7#diff-171571dcd7d2b8608173bb1b312b6e4eR187

Comment 14

[[e-mail], [e-mail]]

Yes, thanks for your point out. But the patch [1] is not uplifted to Gaia/v2.1. I have comment in bug 1043556.(https://bugzilla.mozilla.org/show_bug.cgi?id=1043556#c17) Let's track the regression via bug 1082935, bug 1043556.