1141832 - [Contacts] secure lock disappears and x overlaps text on import contact if viewed from landscape and then switched to portrait.

Status: VERIFIED FIXED

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

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Attachment: 2015-03-10-16-52-54.png

I found a related bug when trying to test a patch :
https://bugzilla.mozilla.org/show_bug.cgi?id=1134009#c37

1) switch orientation to landscape
2) launch contacts
3) go to settings > Impport Contacts > Gmail
4) switch orientation to portrait

Expected: lock should be there and the text should be readable
Actual: lock is not there and the X overlaps the text


Build ID               20150310010227
Gaia Revision          943c8b4039f59b08ba100390e164a076a20c892e
Gaia Date              2015-03-10 20:35:07
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/6686aacf006f
Gecko Version          39.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150310.043222
Firmware Date          Tue Mar 10 04:32:31 EDT 2015
Bootloader             L1TC000118D0

Comment 1

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

qawanted to see if it happens in 2.2 and if so, please nom for 2.2?

Comment 2

[Yeojin Chung [:YeojinC]]

Attachment: Overlapping.png

Without the patch, the issue still reproduces on Flame Master and 2.2, but it looks a little different. The lock icon is still shown, but X is overlapping with the lock and the text. Please see the attached screenshot.

Environmental Variables:
Device: Flame Master
Build ID: 20150310055519
Gaia: f6a1fcd30ee6a286f3bca9d0c3cb600e21bfbf69
Gecko: c42e7e3bb0a0
Version: 39.0a1 (Master)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

Environmental Variables:
Device: Flame 2.2
Build ID: 2015310140620
Gaia: 5af6f8d5d6161dea02002634c6d0a570a122e5dd
Gecko: ec87adb8cf13
Version: 37.0 (2.2)
Firmware Version: v1D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

------------------------------------
This issue does not reproduce on Flame 2.1.

Result: No overlapping or graphic issue is observed.

Environmental Variables:
Device: Flame 2.1
Build ID: 20150309232119
Gaia: 9f2f64bbc9f29edb36f8cbf57d4e0c3f4dee3124
Gecko: c78eec29e8c1
Version: 34.0 (2.1)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 3

[Shine(Leave from Mozilla)]

I'm finding the broken window.

Comment 4

[Shine(Leave from Mozilla)]

b2g-inbound regression window:

Last Working Environmental Variables:
Device: Flame
Gaia-Rev        e846bc37ade8eaad82a00ffda6eb34906dbcc07f
Gecko-Rev       https://hg.mozilla.org/integration/b2g-inbound/rev/20ae4196ad67
Build-ID        20150210011855
Version         38.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150310.192951
FW-Date         Tue Mar 10 19:30:01 EDT 2015
Bootloader      L1TC000118D0

First Broken Environmental Variables:
Device: Flame
Gaia-Rev        8cade57c020f952bfe561c76f4afbcc51029e25a
Gecko-Rev       https://hg.mozilla.org/integration/b2g-inbound/rev/876dde5c2924
Build-ID        20150210020821
Version         38.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150310.192951
FW-Date         Tue Mar 10 19:30:01 EDT 2015
Bootloader      L1TC000118D0

First Broken Gaia & Last Working Gecko - issue DOES repro
Gaia: 8cade57c020f952bfe561c76f4afbcc51029e25a
Gecko: 20ae4196ad67

First Broken Gecko & Last Working Gaia - issue does NOT repro
Gaia: e846bc37ade8eaad82a00ffda6eb34906dbcc07f
Gecko: 876dde5c2924

Comment 5

[Julien Wajsberg [:julienw]]

See also bug 1137097.

Comment 6

[Kai-Chih Hu [:khu]]

Alive, is this issue similar to bug 1137097? 
Thanks.

Comment 7

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

Maybe...I am not sure; but this one is either falling under web component or systemsFE's eye.

Comment 8

[howie [:howie]]

So Gregor, maybe you can find someone to look at this?

Comment 9

[Julien Wajsberg [:julienw]]

I think you'll want to wait until the work in bug 1140613 is finished.

Comment 10

[Gregor Wagner [:gwagner]]

(In reply to howie [:howie] from comment #8)
> So Gregor, maybe you can find someone to look at this?

Should be fixed by 1140613.

Comment 11

[Hermes Cheng (inactive after July 27, 2015)]

Attachment: landscape mode without lock icon.jpg

(In reply to Gregor Wagner [:gwagner] from comment #10)
> (In reply to howie [:howie] from comment #8)
> > So Gregor, maybe you can find someone to look at this?
> 
> Should be fixed by 1140613.

I cannot reproduce this issue.
However, I cannot see the lock icon when turning phone to landscape mode.

Comment 12

[Julien Wajsberg [:julienw]]

I think we should redo what was done in bug 1134009 using the new functionality added in bug 1140613. This will work better (likely fixing the issue from comment 11) and more reliably over time. Sorry for this :/

Basically gaia-header now imports every children; before it was importing only <a>s and <button>s. They are laid out using flex box. So this makes everything easier. Hope this helps.

Comment 13

[Hermes Cheng (inactive after July 27, 2015)]

(In reply to Julien Wajsberg [:julienw] from comment #12)
> I think we should redo what was done in bug 1134009 using the new
> functionality added in bug 1140613. This will work better (likely fixing the
> issue from comment 11) and more reliably over time. Sorry for this :/
> 
> Basically gaia-header now imports every children; before it was importing
> only <a>s and <button>s. They are laid out using flex box. So this makes
> everything easier. Hope this helps.

Julien, did you mean open another bug for what you proposed solution?

Comment 14

[Julien Wajsberg [:julienw]]

(In reply to Hermes Cheng[:hermescheng] from comment #13)
> (In reply to Julien Wajsberg [:julienw] from comment #12)
> > I think we should redo what was done in bug 1134009 using the new
> > functionality added in bug 1140613. This will work better (likely fixing the
> > issue from comment 11) and more reliably over time. Sorry for this :/
> > 
> > Basically gaia-header now imports every children; before it was importing
> > only <a>s and <button>s. They are laid out using flex box. So this makes
> > everything easier. Hope this helps.
> 
> Julien, did you mean open another bug for what you proposed solution?

Why do you want to open another bug while this bug is still open ? :)

Comment 15

[Julien Wajsberg [:julienw]]

I can try to handle this starting tomorrow.

Comment 16

[GitHub Autolander Bot]

Attachment: [gaia] julienw:1141832-secure-lock > mozilla-b2g:master

Comment 17

[Julien Wajsberg [:julienw]]

This is a WIP. The RTL case is not working properly. I also saw that the ellipsis are not really well located when the title is LTR in a RTL phone. I'll try to fix this as well.

Comment 18

[Julien Wajsberg [:julienw]]

Comment on attachment 8586883 [details] [review]
[gaia] julienw:1141832-secure-lock > mozilla-b2g:master

Hey Etienne, Kevin,

I ask from both of you in case Kevin can still take it ;)

I first reverted the commit from bug 1134009 so that I could apply the work done for bug 1140613. We should have done this before but well...

Known issues:
* when the header has a title too big, when there is an ellipsis, the ellipsis is placed relatively to the main document direction, not relatively to the content. This is because gaia-header assumes the title's content has the same direction that the document. However this is not true when the title's has a dynamic content, which is the case here.
  In the SMS app, we use [1] that we put on a <bdi> element. Tell me I should do this here as well, or if you already have something similar in the system app we could use. This could be in a separate bug as well.

* when moving from LTR to RTL, the title's h1 has a little less available space; as a result if the app chrome is displayed with a long title, and we move from LTR to RTL, it could be displayed with an ellipsis because the gaia-header font fit algorithm does not run in that specific case. We could probably do something to fix it but I think it would bring some complexity for not much use. Tell me what you think !

[1] https://github.com/mozilla-b2g/gaia/blob/39f87c403bb068b18a7c5de215bd7282a7eabe1f/apps/sms/style/sms.css#L1062-L1070

Otherwise I think it works well for all other cases.

Tell me what you think :)

Comment 20

[Etienne Segonzac (:etienne)]

Comment on attachment 8586883 [details] [review]
[gaia] julienw:1141832-secure-lock > mozilla-b2g:master

Leaving this one to Kevin since I'm mostly discovering the code and the patch reversed :)

Comment 21

[Kevin Grandon :kgrandon]

Comment on attachment 8586883 [details] [review]
[gaia] julienw:1141832-secure-lock > mozilla-b2g:master

This looks good to me, thank you very much for writing a patch for this. It does make me a bit sad that we lose some testability, but the fix is good.

I left a comment on github which you should address before landing. To reproduce it, load a HTTPS website in the browser, then minimize the statusbar by scrolling down - you should see the SSL icons in the minimized statusbar which should not happen.

Comment 22

[Julien Wajsberg [:julienw]]

Thx, I will definitely fix this.

Comment 23

[Julien Wajsberg [:julienw]]

Fixed Kevin's comment and it works fine in all cases I tried.

Checkin-needed

Comment 24

[Julien Wajsberg [:julienw]]

Comment on attachment 8586883 [details] [review]
[gaia] julienw:1141832-secure-lock > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): bug 1134009 and bug 1112131
[User impact] if declined: Bad
[Testing completed]: yes, extensively by hand
[Risk to taking this patch] (and alternatives if risky): fairly low
[String changes made]: none

Comment 25

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/cb168811c9ed6b1ebf143aba6eda321b5b1cbdd8

Comment 26

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/e547fd9a1eb4a95ee2468cd4deef3ebcb5eadb12

Comment 27

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

On the latest nightly 3.0 and 2.2 Flame builds, the Lock icon and X are both present and do not overlap the text.  However during the first transition from landscape to portrait the icons will overlap for a short period of time.  
	
This issue only occurs once after the phone has been turned on, and subsequent attempts at the same steps will show a smooth transition.

Hermes should this be written up separately and this issue fixed or marked as a failure to fix this issue?

Environmental Variables:
Device: Flame 3.0
BuildID: 20150408010203
Gaia: 84cbd4391fb7175d5380fa72c04d68873ce77e6d
Gecko: 078128c2600a
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Environmental Variables:
Device: Flame 3.0
BuildID: 20150408010203
Gaia: 84cbd4391fb7175d5380fa72c04d68873ce77e6d
Gecko: 078128c2600a
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Comment 28

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

The second set of variables for comment 27 was supposed to be the ones below.  Sorry for any confusion. 

Environmental Variables:
Device: Flame 2.2
BuildID: 20150408002503
Gaia: ea735c21bfb0d78333213ff0376fce1eac89ead6
Gecko: 43041c78052b
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Comment 29

[Hermes Cheng (inactive after July 27, 2015)]

Hi Jayme,

I cannot reproduce what you mentioned for now, but I think you should open a separate bug and refer to this one as its possible cause. Thanks.

Comment 30

[Julien Wajsberg [:julienw]]

(In reply to Jayme Mercado [:JMercado] from comment #27)
> On the latest nightly 3.0 and 2.2 Flame builds, the Lock icon and X are both
> present and do not overlap the text.  However during the first transition
> from landscape to portrait the icons will overlap for a short period of
> time. 

What do you mean, the "icons will overlap" ? Can you try to be more precise (possibly with a video) in the new bug ? Thanks !

Comment 31

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

I can't reproduce what I was seeing anymore either on today's build.  I will check a few more times over the next few days.  What I meant was that the Lock icon and X would overlap the 'Sign in - Google Accounts' text very briefly during the transition from landscape to portrait mode.  I will mark this issue as verified and if I can reproduce that issue again I will write it up with a video.

Environmental Variables:
Device: Flame 3.0
Build ID: 20150409010203
Gaia: 5dfd0460eb6e616205154b0d219aa5123bf1abb3
Gecko: 8f57f60ee58a
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Comment 32

[Julien Wajsberg [:julienw]]

I wouldn't be surprised that this could happen because we recalculate the font size a slight time after the resize happens. I dont think we'd have any way to fix this properly though :/