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

VERIFIED FIXED in 2.2 S10 (17apr)

Status

defect
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: nhirata, Assigned: julienw)

Tracking

({regression})

unspecified
2.2 S10 (17apr)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.1 unaffected, b2g-v2.2 verified, b2g-master verified)

Details

Attachments

(4 attachments)

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
qawanted to see if it happens in 2.2 and if so, please nom for 2.2?
Keywords: qawanted
QA Contact: ychung
Posted image 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
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?]
Keywords: qawanted
QA Contact: ychung
blocking-b2g: --- → 2.2?
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
I'm finding the broken window.
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
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][MGSEI-Triage+]
Depends on: 1134009
blocking-b2g: 2.2? → 2.2+
(Assignee)

Comment 5

4 years ago
See also bug 1137097.
(Assignee)

Updated

4 years ago
See Also: → 1137097
Depends on: 1137097
Alive, is this issue similar to bug 1137097? 
Thanks.
Flags: needinfo?(alive)
Maybe...I am not sure; but this one is either falling under web component or systemsFE's eye.
Flags: needinfo?(alive)

Comment 8

4 years ago
So Gregor, maybe you can find someone to look at this?
Flags: needinfo?(anygregor)
(Assignee)

Comment 9

4 years ago
I think you'll want to wait until the work in bug 1140613 is finished.
(In reply to howie [:howie] from comment #8)
> So Gregor, maybe you can find someone to look at this?

Should be fixed by 1140613.
Flags: needinfo?(anygregor)
Whiteboard: [waiting on 1140613]

Updated

4 years ago
Depends on: 1140613
(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.
(Assignee)

Comment 12

4 years ago
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.
Whiteboard: [waiting on 1140613]
(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?
Flags: needinfo?(felash)
(Assignee)

Comment 14

4 years ago
(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 ? :)
Flags: needinfo?(felash)
(Assignee)

Comment 15

4 years ago
I can try to handle this starting tomorrow.
Assignee: nobody → felash
(Assignee)

Comment 17

4 years ago
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.
(Assignee)

Comment 18

4 years ago
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 :)
Attachment #8586883 - Flags: review?(kgrandon)
Attachment #8586883 - Flags: review?(etienne)
(Assignee)

Updated

4 years ago
Duplicate of this bug: 1150426
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 :)
Attachment #8586883 - Flags: review?(etienne)
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.
Attachment #8586883 - Flags: review?(kgrandon) → review+
(Assignee)

Comment 22

4 years ago
Thx, I will definitely fix this.
(Assignee)

Comment 23

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

Checkin-needed
Keywords: checkin-needed
(Assignee)

Comment 24

4 years ago
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
Attachment #8586883 - Flags: approval-gaia-v2.2?
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Attachment #8586883 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
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
QA Whiteboard: [QAnalyst-Triage+][MGSEI-Triage+] → [QAnalyst-Triage?][MGSEI-Triage+]
Flags: needinfo?(ktucker)
Flags: needinfo?(hcheng)
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
QA Whiteboard: [QAnalyst-Triage?][MGSEI-Triage+] → [QAnalyst-Triage+][MGSEI-Triage+]
Flags: needinfo?(ktucker)
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.
Flags: needinfo?(hcheng) → needinfo?(jmercado)
(Assignee)

Comment 30

4 years ago
(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 !
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
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+][MGSEI-Triage+] → [QAnalyst-Triage?][MGSEI-Triage+]
Flags: needinfo?(jmercado) → needinfo?(ktucker)
Keywords: verifyme
QA Whiteboard: [QAnalyst-Triage?][MGSEI-Triage+] → [QAnalyst-Triage+][MGSEI-Triage+]
Flags: needinfo?(ktucker)
(Assignee)

Comment 32

4 years ago
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 :/
You need to log in before you can comment on or make changes to this bug.