914362 - [B2G][Buri 1.2][Bluetooth]Long device names offset Bluetooth Request Form PIN

Status: RESOLVED FIXED

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

Description

[gbennett]

Attachment: 2013-09-09-13-58-07.png

Description:
After trying to pair to a device with a name equal or longer than "ALCATEL ONE TOUCH FIRE", the Bluetooth Request Form PIN will be overlapping other text.

Repro Steps:
1) Updated Buri 1.2.0 to Build ID: 20130906040204
2) In the phone's video player, or from the gallery, select a video file.
3) Click "share" or share icon.
4) Select "Bluetooth Transfer" option.
5) Rename your device to "ACATEL ONE TOUCH FIRE"
6) Choose the device to which you want to transfer the files.

Actual:
PIN number is overlapping Bluetooth Request Form description.

Expected:
No strings overlap one another.

Environmental Variables
Device: Buri 1.2.0
Build ID: 20130906040204
Gecko: http://hg.mozilla.org/mozilla-central/rev/ab5f29823236
Gaia: 94e5f269874b02ac0ea796b64ab995fce9efa4b3
Platform Version: 26.0a1

Notes: This was found running test case 6753: https://moztrap.mozilla.org/manage/cases/?filter-id=6753#caseversion-id-50194
Repro frequency: 100%, 3/3
See attached: 2013-09-09-13-58-07.png

Comment 1

[Eric Chou [:ericchou] [:echou]]

This is an UX/UI issue. Reset the target component.

Comment 2

[Eric Chou [:ericchou] [:echou]]

This may be another good-first-bug.

Comment 3

[sarsenyev]

All tested 1.2 builds are affected 
The issue is reproduced from the first 1.2 build on 6/21
The 1.1 builds aren't affected

Environmental  Variables:
Build ID: 20130621031231
Gecko: http://hg.mozilla.org/mozilla-central/rev/7ba8c86f1a56
Gaia: e2f19420fa6a26c4287588701efaec09a750dba1
Platform Version: 24.0a1

Comment 4

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Attachment: screenshot.png

Hi all,

I just updated the code to show part of our device name instead of showing them all. Not sure this is OK for UX or not ?

Comment 5

[Eric Chou [:ericchou] [:echou]]

I think you need UX input in this case. Let's cc Carrie, who is in charge of Bluetooth UX.

Comment 6

[Ivan Tsay (:ITsay)]

Visual detail. Minus this bug since it is not blocker. But it can be uplifted to v1.2 by approval once it is fixed.

Comment 7

[Carrie Wang [New account: [e-mail]]]

Hi all,

I've uploaded two attachments. 
Our visual designer has relayout the case that we faced here (base on design guideline). Please take a look!
However, if the device name contains more than 38 letters (more than 2 lines), then we just show part of the device name. Thanks!

Hi Eric, 

Please confirm our change here. Thanks!

Comment 8

[Carrie Wang [New account: [e-mail]]]

Attachment: bluetooth pairing_02.png

Comment 9

[Carrie Wang [New account: [e-mail]]]

Attachment: bluetooth pairing_1.png

Comment 10

[Eric Pang [:epang] UX]

(In reply to cawang from comment #8)
> Created attachment 809784 [details]
> bluetooth pairing_02.png

This looks good, thanks Carrie!

Comment 11

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Attachment: Pointer to Github pull request 12467.html

Hi Arthur, 

I just updated the code and got Carries's confirmation about this change. 

Please help me review the code when you are free. 

Thanks for your great help :P

Comment 12

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 810460 [details]
Pointer to Github pull request 12467.html

redirect to the bugzilla coount.

Comment 13

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 810460 [details]
Pointer to Github pull request 12467.html

EJ, thank you for the patch! Please check my comments in github.

Comment 14

[Jason Smith [:jsmith]]

Could someone find out if this reproduces on 1.1?

Comment 15

[Jason Smith [:jsmith]]

(In reply to Ivan Tsay (:ITsay) from comment #6)
> Visual detail. Minus this bug since it is not blocker. But it can be
> uplifted to v1.2 by approval once it is fixed.

You need to confirm first if it's a regression or not. Visual detail isn't a justification for non-blocking always. That really only applies on polish bugs (i.e. existing UX already works fine but improving it can improve the visual appearance). This is broken UX that prevents readability of the target message in the presented UX, which is broken visual design. If it's a regression from a past release, then it indeed needs to block, as we cannot slip quality from past releases.

Comment 16

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 810460 [details]
Pointer to Github pull request 12467.html

Hi Arthur,

I just implemented a method called getTruncated() in utils.js and it works well. I will move that into shared later in the future ! Please give it a try !

Comment 17

[Natalya Kot [:nkot]]

(In reply to Jason Smith [:jsmith] from comment #14)
> Could someone find out if this reproduces on 1.1?

Does not reproduce on v1.1

Buri
BuildID: 20131010041201
Gaia: 53e2a70d85fb3748d0768218a5efffe5806073f0
Gecko: cb2a1a27a94c
Version: 18.0

Comment 18

[Jason Smith [:jsmith]]

(In reply to nkot from comment #17)
> (In reply to Jason Smith [:jsmith] from comment #14)
> > Could someone find out if this reproduces on 1.1?
> 
> Does not reproduce on v1.1
> 
> Buri
> BuildID: 20131010041201
> Gaia: 53e2a70d85fb3748d0768218a5efffe5806073f0
> Gecko: cb2a1a27a94c
> Version: 18.0

Okay, that confirms this is a regression. comment 15's analysis then applies, which makes this a blocker.

Comment 19

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 810460 [details]
Pointer to Github pull request 12467.html

EJ, thanks for the patch. Please check my comments in github.

Comment 20

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 810460 [details]
Pointer to Github pull request 12467.html

Hi Arthur, I rewrote the getTruncated function and tested with many combinations with numbers, English and Chinese.

By the way, I will rename `parentNode` later to appropriate naming and if you find any inappropriate naming, just comment on Github and I will change that later. 

Thanks.

Comment 21

[Arthur Chen [:arthurcc][inactive after 6/30]]

Attachment: 2013-10-21-10-57-54.png

The passcode layout is broken on my device as the attachment. Could you take a look at it? Thanks.

Comment 22

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

(In reply to Arthur Chen [:arthurcc] from comment #21)
> Created attachment 819525 [details]
> 2013-10-21-10-57-54.png
> 
> The passcode layout is broken on my device as the attachment. Could you take
> a look at it? Thanks.

Arthur, this may be l10n/UX bug. our logic is right but when buri is asked to pair with the other phone, the l10n string would be too long to fit the container. We can change UI or l10n strings (I don't recommend truncating our "truncated" device name, because this will make out UI inconsistent). 

Any good idea ?

Comment 23

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 810460 [details]
Pointer to Github pull request 12467.html

Thanks for the effort! Per our discussion, please check if the simpler way is sufficient for the task.

Comment 24

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 810460 [details]
Pointer to Github pull request 12467.html

Arthur, thanks for your reviewing first. I just tried a better way to implement this feature. 

Following our discussed way to implement this feature will need a second check for a situation that we still have room for deviceName. (If we don't do a second check, we will mistakenly treat the deviceName as truncate-able in boundary case)

Not sure this part of logic would make more sense to you, so if you have any problem about this new logic, we can discuss more about that.

Big Thanks :D

Comment 25

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 810460 [details]
Pointer to Github pull request 12467.html

Thanks for the effort. r=me with all nits in github addressed

Comment 26

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Thanks @Arthur. 

Landed on master, commit : 098b4a0f426a1bc8cae5d1cf8f7c53392ab95743

Comment 27

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Landed on v1.2, commit : 1ca623e7a79a9653b54a45ef7f8bb03b8d7100fc

Comment 28

[John Ford [:jhford] CET/CEST Berlin Time]

Please remember to set status flags

Comment 29

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

@John, I didn't realize I have to set that flag. Thanks for your great help, I will keep that in mind :)