1225047 - [Contacts] The contacts/ICE suggestion bar Cancel button will show as a rectangular button.

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::Dialer, defect, P1)

Description

[Queena.Wen(Leave from Mozilla)]

Attachment: logcat_1815.txt

[1.Description]:
[Aries KK V2.5&2.6][Flame KK V2.5&2.6] The Cancel button will show as a rectangular button which  is similar to other options in the menu, with its text left-aligned and there is a blank space below it at the bottom of the screen.
See attachments: Aries_KK_V2.5. 3gp&Contrast. png&logcat_1815.txt
Found time: 18:15

[2.Testing Steps]: 
The precondition: There is an ICE Contact and device is locked with passwored.
1. Press the power button
2. Slide to the right
3. Tap the "EMERGENCY CALL"
4. Tap the "ICE Contacts"

[3.Expected Result]: 
4. The "Cancel" button should show as an oval button.

[4.Actual Result]: 
4. The Cancel button will show as a rectangular button similar to other options in the menu, with its text left-aligned and there is a blank space below it at the bottom of the screen.

[5.Reproduction build]: 
Device: Aries KK v2.5(Affected)
Build ID               20151113224539
Gaia Revision          142a25e39196f036978e0dd6c94640bd8d4d692a
Gaia Date              2015-11-12 18:38:22
Gecko Revision         http://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/6f13164b9a2922000c04e641165fd8ce3c5dec8d
Gecko Version          44.0a2
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20151113.215500
Firmware Date          Fri Nov 13 21:55:07 UTC 2015
Bootloader             s1

Device: Flame KK v2.5 512mb(Affected)
Build ID               20151113224011
Gaia Revision          142a25e39196f036978e0dd6c94640bd8d4d692a
Gaia Date              2015-11-12 18:38:22
Gecko Revision         http://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/6f13164b9a2922000c04e641165fd8ce3c5dec8d
Gecko Version          44.0a2
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20151113.215220
Firmware Date          Fri Nov 13 21:52:29 UTC 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

Device: Aries KK v2.6 master(Affected)
Build ID               20151112120735
Gaia Revision          27bc9412ca607648bc398b25bb1ae25653b2b278
Gaia Date              2015-11-12 02:27:47
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/3cc3b1968524248450c465c4ea2ee5596ffa65f2
Gecko Version          45.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20151112.112635
Firmware Date          Thu Nov 12 11:26:43 UTC 2015
Bootloader             s1

Device: Flame KK v2.6 master 512mb(Affected)
Build ID               20151115150212
Gaia Revision          e8c15ae4e5324a210000ee0a869a962aa542009f
Gaia Date              2015-11-13 00:10:48
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/7cd2d806bd069c0260ff73f023ac85f892b863bf
Gecko Version          45.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20151115.184416
Firmware Date          Sun Nov 15 18:44:31 EST 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

[6.Reproduction Frequency]: 
Always Recurrence,10/10

[7.TCID]: 
Free Test

Comment 1

[Queena.Wen(Leave from Mozilla)]

Attachment: Aries_KK_V2.5.3gp

Comment 2

[Queena.Wen(Leave from Mozilla)]

Attachment: Contrast.png

Comment 3

[Isabel Rios[:isabel_rios]]

[Blocking Requested - why for this release]: This breaks the UX consistency

Comment 4

[GitHub Autolander Bot]

Attachment: [gaia] KevinGrandon:bug_1225047_contacts_gaia_menu > mozilla-b2g:master

Comment 5

[Kevin Grandon :kgrandon]

Comment on attachment 8688267 [details] [review]
[gaia] KevinGrandon:bug_1225047_contacts_gaia_menu > mozilla-b2g:master

I'm not sure who does dialer stuff any more. Etienne - would you be able to review this? Thanks!

Comment 6

[Etienne Segonzac (:etienne)]

Comment on attachment 8688267 [details] [review]
[gaia] KevinGrandon:bug_1225047_contacts_gaia_menu > mozilla-b2g:master

Gabriele does, but looking good :)

Comment 7

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8688267 [details] [review]
[gaia] KevinGrandon:bug_1225047_contacts_gaia_menu > mozilla-b2g:master

First of all I'm really glad to see that particular overlay go and be replaced by shared code. The dialer is one of the most offenders as far as custom overlays go, in fact we even have a bug just for that (bug 1134631) so shooting down one is great.

Overall the patch looks good but I've spotted a visual issue it introduces: the buttons containing the contacts go from being 62px tall to only 40px tall which causes the contents to barely fit. I'll attach a screenshot with the issue highlighted.

Comment 8

[Gabriele Svelto [:gsvelto]]

Attachment: Review screenshot

Comment 9

[Johan Lorenzo [:jlorenzo]]

Comms triage: Visual regression that might lead to user's confusion.

Comment 10

[Kevin Grandon :kgrandon]

Comment on attachment 8688267 [details] [review]
[gaia] KevinGrandon:bug_1225047_contacts_gaia_menu > mozilla-b2g:master

Hi Gabriele,

That was working previously via a CSS override, which does not work too well with web components. A better fix is to probably move that into the web component itself, but I think it'd be better to take as-is and fix the web component to handle multiple line buttons later. I had to unfortunately add !important to be able to override. Please take another look.

Comment 11

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8688267 [details] [review]
[gaia] KevinGrandon:bug_1225047_contacts_gaia_menu > mozilla-b2g:master

Agreed, please open a follow up bug and put a comment above the ! rules referring to it so that we know that should be fixed.

Comment 12

[Kevin Grandon :kgrandon]

Thanks! In master: https://github.com/mozilla-b2g/gaia/commit/0a7619882362737562e79fc87226e08632d0c3d9

Comment 13

[Kevin Grandon :kgrandon]

Comment on attachment 8688267 [details] [review]
[gaia] KevinGrandon:bug_1225047_contacts_gaia_menu > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Platform change requiring UI updates.
[User impact] if declined: Poor UX in menu.
[Testing completed]: Manual testing (porting to web component)
[Risk to taking this patch] (and alternatives if risky): Should be low because we're porting to a common web component.
[String changes made]: None.

Comment 14

[Mahendra Potharaju [:mahe]]

Comment on attachment 8688267 [details] [review]
[gaia] KevinGrandon:bug_1225047_contacts_gaia_menu > mozilla-b2g:master

Approved for 2.5 Uplift. 

Thanks

Comment 15

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

The changes here caused the smoketest blocker bug 1226308.

Comment 16

[Kevin Grandon :kgrandon]

Because that is a smoketest blocker, and this needs to be uplifted, let's backout and fix it.

Comment 17

[Kevin Grandon :kgrandon]

Backed out to fix smoketest blocker:

https://github.com/mozilla-b2g/gaia/commit/20f140d8b0142a151ff58f726782526c6b2354c9

Submitting a new patch /w a fix.

Comment 18

[GitHub Autolander Bot]

Attachment: [gaia] KevinGrandon:bug_1225047_dialer_gaia_menu_usage > mozilla-b2g:master

Comment 19

[Kevin Grandon :kgrandon]

Comment on attachment 8689908 [details] [review]
[gaia] KevinGrandon:bug_1225047_dialer_gaia_menu_usage > mozilla-b2g:master

Gabriele - I'm sorry, I caused a smoketest blocker the last time this landed.

In this patch I removed the hideOverlay function of suggestionBar as that accessibility code can be handled from within the component now. I forgot to search for all references of that, and there was one left in dialer.js. I believe this is what was causing the smoketest failure.

I've updated this with a the updated call to `SuggestionBar.overlay.hide`[1]. Please let me know what you think. Thanks and sorry for the noise.

[1] https://github.com/mozilla-b2g/gaia/pull/33287/files#diff-660f9c7d029624283424917265eb27f5R352

Comment 20

[Gabriele Svelto [:gsvelto]]

I've tried the STR of bug 1226308 with your new patch but the issue is still there. I'm not quite sure what's going on, I'll try to investigate further next week.

Comment 21

[Kevin Grandon :kgrandon]

Ok, thanks. I'm having trouble getting a sim where I can actually enable the pin code. I'll take another look though.

Comment 22

[Gabriele Svelto [:gsvelto]]

OK, I've figured it out, the hide() method is undefined here:

https://github.com/mozilla-b2g/gaia/pull/33287/files#diff-660f9c7d029624283424917265eb27f5R352

I'm not sure why this is, possibly the code hasn't been lazy-loaded yet or the component's methods haven't been added to that object?

Comment 23

[Gabriele Svelto [:gsvelto]]

(In reply to Kevin Grandon :kgrandon from comment #21)
> Ok, thanks. I'm having trouble getting a sim where I can actually enable the
> pin code. I'll take another look though.

If you've got some patience you can use the emulator, changing the SIM PIN works there (note that the emulated SIM has a 0000 PIN number baked in by default).

I recommend the following build, it's real fast if you've got VT-d enabled on your machine (FxOS runs natively) and doesn't take up much space on disk:

git clone https://github.com/mozilla-b2g/B2G.git B2G-emulator-x86-kk
cd B2G-emulator-x86-kk
env REPO_INIT_FLAGS="--depth=1" ./config.sh emulator-x86-kk
./build.sh

Comment 24

[Kevin Grandon :kgrandon]

Thank you for the info on the emulator, I will test in there to make sure I get it. I wonder if we can also write a marionette test for this.

Comment 25

[Kevin Grandon :kgrandon]

Removing keyword because this is already backed out.

Comment 26

[Kevin Grandon :kgrandon]

Comment on attachment 8689908 [details] [review]
[gaia] KevinGrandon:bug_1225047_dialer_gaia_menu_usage > mozilla-b2g:master

Clearing request until it's confirmed working.

Comment 27

[Carsten Book [:Tomcat]]

(In reply to Mahendra Potharaju [:mahe] from comment #14)
> Comment on attachment 8688267 [details] [review]
> [gaia] KevinGrandon:bug_1225047_contacts_gaia_menu > mozilla-b2g:master
> 
> Approved for 2.5 Uplift. 
> 
> Thanks

mahe, do you really want this uplifted, seems this got backed out and there is work going on here ?

Comment 28

[Kevin Grandon :kgrandon]

Comment on attachment 8688267 [details] [review]
[gaia] KevinGrandon:bug_1225047_contacts_gaia_menu > mozilla-b2g:master

Let's not uplift until it sticks. Thanks.

Comment 29

[Mahendra Potharaju [:mahe]]

Thanks for watching this, Tomcat. I approved it before the back out.

Comment 30

[Kevin Grandon :kgrandon]

(In reply to Gabriele Svelto [:gsvelto] from comment #23)
> I recommend the following build, it's real fast if you've got VT-d enabled
> on your machine (FxOS runs natively) and doesn't take up much space on disk:
> 
> git clone https://github.com/mozilla-b2g/B2G.git B2G-emulator-x86-kk
> cd B2G-emulator-x86-kk
> env REPO_INIT_FLAGS="--depth=1" ./config.sh emulator-x86-kk
> ./build.sh

I still want to work on this, but I'm having trouble building emulators on OSX 10.11 right now, seems it's not very well supported =( Seeing if there's another way I can test (I don't want to mess up my Foxfooding sim card)

Comment 31

[Gabriele Svelto [:gsvelto]]

(In reply to Kevin Grandon :kgrandon from comment #30)
> I still want to work on this, but I'm having trouble building emulators on
> OSX 10.11 right now, seems it's not very well supported =( Seeing if there's
> another way I can test (I don't want to mess up my Foxfooding sim card)

Unfortunately I don't think the emulator works well on Mac, I've always used it under Linux and if you want to make it go fast it must be native Linux with virtualization support. You could try the plain ARM emulator (emulator-kk config target). It's going to be real slow, but it should work.

Comment 32

[Kevin Grandon :kgrandon]

Comment on attachment 8689908 [details] [review]
[gaia] KevinGrandon:bug_1225047_dialer_gaia_menu_usage > mozilla-b2g:master

(In reply to Gabriele Svelto [:gsvelto] from comment #22)
> OK, I've figured it out, the hide() method is undefined here:

Hi Gabriele, 

I haven't gotten my emulator building still, but I'm working on it and was wondering if you'd be willing to review this again in the meantime. (I do want to get this working though and will keep working on it)

My hunch about the problem is that the element is not upgraded to a web component because it does not get inserted into the document - so the methods are not available yet. I've added a simple guard for this here:

https://github.com/mozilla-b2g/gaia/pull/33287/files#diff-660f9c7d029624283424917265eb27f5R352

I don't want to waste your time, so if this doesn't work - I'll wait until I can fully verify this myself. Thanks!

Comment 33

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8689908 [details] [review]
[gaia] KevinGrandon:bug_1225047_dialer_gaia_menu_usage > mozilla-b2g:master

I just gave it a spin and it still doesn't work :(

Comment 34

[Gabriele Svelto [:gsvelto]]

Changing the title to reflect that the fix also affects the regular suggestion bar.

Comment 36

[BMO Automation]

Firefox OS is not being worked on