[Flame][Video] the option (...) button is not responsive on the first touch

VERIFIED FIXED in 2.1 S6 (10oct)

Status

VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: njpark, Assigned: pdahiya)

Tracking

({regression})

unspecified
2.1 S6 (10oct)
ARM
Gonk (Firefox OS)
regression
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(blocking-b2g:2.1+, b2g-v2.1 verified, b2g-v2.2 verified)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

4 years ago
STR: 
Open video app, select a video
Pause the video, and touch screen to show overlay
Tap option (...) button

Actual:
the option menu shows up after 3~4 taps

Expected:
The option menu shows up on first tap

Version info:
Gaia-Rev        86a3626055ed58b39525424126870dc0a503e79f
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-aurora/rev/c20912812877
Build-ID        20141001000202
Version         34.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141001.093448
FW-Date         Wed Oct  1 09:35:01 EDT 2014
Bootloader      L1TC10011800

This does not reproduce on 319MB 2.0 Flame.

Video link:
https://www.dropbox.com/s/cuks9vwp2ovlm1z/VID_unresponsiveOptionbtn.mp4?dl=0
(Reporter)

Comment 1

4 years ago
[Blocking Requested - why for this release]:
Noticeably bad UI experience
blocking-b2g: --- → 2.1?
status-b2g-v2.1: --- → affected
status-b2g-v2.2: --- → affected
Keywords: regression

Comment 2

4 years ago
Hema, Can you assign someone to analyze this..
Flags: needinfo?(hkoka)
Similar bug I filed: Bug 1075115

Comment 4

4 years ago
Blocking Reason: options menu show up on first tap, other wise it will lead to user frustration (looks like regression)

Punam, Can you check whats going on?

Thanks
Hema
Assignee: nobody → pdahiya
blocking-b2g: 2.1? → 2.1+
Flags: needinfo?(hkoka)
(Assignee)

Comment 5

4 years ago
Able to replicate the issue on 2.2 flame kk buildId:20141001040205, sometimes it take 2-3 taps to open options view, will try to increase options button touch are and see if that fixes the issue
Blocks: 1069288
(Assignee)

Comment 6

4 years ago
Created attachment 8499861 [details] [review]
PR with fix of bug 1076158

Hi Wilson

As discussed, I am attaching the PR that moves options icon inside gaia-header 10px more to left so that touch area doesn't conflict with edge-gestures. Please review. Thanks
Attachment #8499861 - Flags: review?(wilsonpage)
(Assignee)

Comment 7

4 years ago
Setting NI flag for No-Jun to help validate attached patch fixes options icon responsiveness issue. Thanks
Flags: needinfo?(npark)
Comment on attachment 8499861 [details] [review]
PR with fix of bug 1076158

Looks good to me. Flagging Amy to make sure this doesn't impact any visual stuff.
Attachment #8499861 - Flags: ui-review?(amlee)
Attachment #8499861 - Flags: review?(wilsonpage)
Attachment #8499861 - Flags: review+
(Reporter)

Comment 9

4 years ago
Tried out the patch, it is definitely more responsive than before.  I can open the options dialog with at most 2 touches, and it feels more responsive.  Once I locate the exact place to touch, then it opens the dialog consistently.
Flags: needinfo?(npark)
(In reply to Wilson Page [:wilsonpage] from comment #8)
> Comment on attachment 8499861 [details] [review]
> PR with fix of bug 1076158
> 
> Looks good to me. Flagging Amy to make sure this doesn't impact any visual
> stuff.

Can you attach a screenshot? Having issues flashing this patch. Thanks
Flags: needinfo?(pdahiya)
(Assignee)

Comment 11

4 years ago
Created attachment 8500678 [details]
Video Options Screen Shot
Flags: needinfo?(pdahiya)
Comment on attachment 8499861 [details] [review]
PR with fix of bug 1076158

The options icon now sits 7px more to the left compared to the other apps that also have an options icon in the header (i.e messages app and camera image preview). Do we need to move the icon over? I'd like to keep the position of the options icon consistent across the apps. If we aren't having any responsive issues with the options icon in other apps, we should keep the position of the options icon the same across apps. 

I would also check to make sure the header transparency/colour in the video player is the same as the header in image preview in camera and that the video player background colour is the same as image preview in camera. If the colour changes are out of scope of this bug then I can file a new one.

Thanks!
Attachment #8499861 - Flags: ui-review?(amlee) → ui-review-
Flags: needinfo?(pdahiya)
(In reply to Amy Lee [:amylee] from comment #12)
> Comment on attachment 8499861 [details] [review]
> PR with fix of bug 1076158
> 
> The options icon now sits 7px more to the left compared to the other apps
> that also have an options icon in the header (i.e messages app and camera
> image preview). Do we need to move the icon over? I'd like to keep the
> position of the options icon consistent across the apps. If we aren't having
> any responsive issues with the options icon in other apps, we should keep
> the position of the options icon the same across apps. 

pdahiya: Is this not any issue in other apps? If not we need to identify what is different in Video app.

> I would also check to make sure the header transparency/colour in the video
> player is the same as the header in image preview in camera and that the
> video player background colour is the same as image preview in camera. If
> the colour changes are out of scope of this bug then I can file a new one.

Yes this is out of scope.
(Assignee)

Comment 14

4 years ago
Created attachment 8501193 [details] [review]
PR updating option icon to button

Thanks Amy for ui-review. I agree we should have option icon consistent in all the apps where used in gaia-header. I have attached PR removing 7px shift to left. 

Attached PR has changed anchor tag to button for option icon. I have tested and seeing responsiveness similar to camera preview option icon (using theme-media class for gaia-header).

No-Juan, Can you pl. test the attached PR and confirm if the option icon responsiveness with this patch is acceptable. Thanks
Attachment #8501193 - Flags: ui-review?(amlee)
Attachment #8501193 - Flags: review?(wilsonpage)
Flags: needinfo?(pdahiya) → needinfo?(npark)
(Assignee)

Comment 15

4 years ago
(In reply to Wilson Page [:wilsonpage] from comment #13)
> (In reply to Amy Lee [:amylee] from comment #12)
> > Comment on attachment 8499861 [details] [review]
> > PR with fix of bug 1076158
> > 
> > The options icon now sits 7px more to the left compared to the other apps
> > that also have an options icon in the header (i.e messages app and camera
> > image preview). Do we need to move the icon over? I'd like to keep the
> > position of the options icon consistent across the apps. If we aren't having
> > any responsive issues with the options icon in other apps, we should keep
> > the position of the options icon the same across apps. 
> 
> pdahiya: Is this not any issue in other apps? If not we need to identify
> what is different in Video app.

wilson: Messaging app is using theme-communications for gaia-header. Camera and Video app are using theme-media, the only difference for Video app was anchor tag for option icon which I have updated in attachment 8501193 [details] [review]. As noted above,  with the patch the responsiveness for video app option icon is similar to camera preview options icon. 


> > I would also check to make sure the header transparency/colour in the video
> > player is the same as the header in image preview in camera and that the
> > video player background colour is the same as image preview in camera. If
> > the colour changes are out of scope of this bug then I can file a new one.
> 
> Yes this is out of scope.
Comment on attachment 8501193 [details] [review]
PR updating option icon to button

Code looks fine. I still understand why this would fix the issue though.
Attachment #8501193 - Flags: review?(wilsonpage) → review+
(Reporter)

Comment 17

4 years ago
(In reply to Punam Dahiya from comment #14)
> Created attachment 8501193 [details] [review]
> PR updating option icon to button
> 
> Thanks Amy for ui-review. I agree we should have option icon consistent in
> all the apps where used in gaia-header. I have attached PR removing 7px
> shift to left. 
> 
> Attached PR has changed anchor tag to button for option icon. I have tested
> and seeing responsiveness similar to camera preview option icon (using
> theme-media class for gaia-header).
> 
> No-Juan, Can you pl. test the attached PR and confirm if the option icon
> responsiveness with this patch is acceptable. Thanks

The icon responsiveness here with this patch is better than before, quite acceptable.
Flags: needinfo?(npark)
(Assignee)

Comment 18

4 years ago
Created attachment 8501217 [details]
Screen Shots video options icon

Attaching screen shot of video app options icon with 8501193 patch. Thanks
Attachment #8499861 - Attachment is obsolete: true
Attachment #8500678 - Attachment is obsolete: true
Punam, do I understand correctly that this new patch just changes the tag name and does not shift the position of the element? If so, then you probably don't even need a UI review for it.  If No-Jun approves, then I'd think this can land without Amy's review.

Thanks for noticing the <a> vs <button> difference between the apps. I wonder if <button> tags get some automatic touch area fluffing that <a> tags do not.  Or if there is some special case in the edge gesture detection code that looks for nearby buttons but not nearby links.
(Assignee)

Comment 20

4 years ago
(In reply to David Flanagan [:djf] from comment #19)
> Punam, do I understand correctly that this new patch just changes the tag
> name and does not shift the position of the element? If so, then you
> probably don't even need a UI review for it.  If No-Jun approves, then I'd
> think this can land without Amy's review.
> 
That's correct, with new patch UI stays same. I will cancel ui-review request for this patch.

> Thanks for noticing the <a> vs <button> difference between the apps. I
> wonder if <button> tags get some automatic touch area fluffing that <a> tags
> do not.  Or if there is some special case in the edge gesture detection code
> that looks for nearby buttons but not nearby links.

There's a slight improvement seen in touch with button tag on top half which was missing with anchor tag.
(Assignee)

Updated

4 years ago
Attachment #8501193 - Flags: ui-review?(amlee)
(Assignee)

Comment 21

4 years ago
Patch landed on master
https://github.com/mozilla-b2g/gaia/commit/4e4bc2051096a6719b9775a90b03a00f7a361ea1
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Assignee)

Comment 22

4 years ago
Comment on attachment 8501193 [details] [review]
PR updating option icon to button

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Bug 1015248 
[User impact] if declined: Slow responsiveness while touching video options
[Testing completed]: On master
[Risk to taking this patch] (and alternatives if risky): None. One line patch that changes anchor tag to use button tag to help increase options icon responsiveness. Patch also removes a redundant closing header tag.

[String changes made]:None
Attachment #8501193 - Flags: approval-gaia-v2.1?(bbajaj)
(Assignee)

Comment 23

4 years ago
Just realized, because of bug 1078536 patch in master attachment 8501193 [details] [review] will not be a straight uplift to 2.1. Here's the link to 2.1 version of the fix. Thanks
https://github.com/mozilla-b2g/gaia/pull/24903
Attachment #8501193 - Flags: approval-gaia-v2.1?(bbajaj) → approval-gaia-v2.1+
v2.1: https://github.com/mozilla-b2g/gaia/commit/cf8cbc192232987958dfc44f298f7ebc00fcc3cc
status-b2g-v2.1: affected → fixed
status-b2g-v2.2: affected → fixed
Target Milestone: --- → 2.1 S6 (10oct)
(Reporter)

Updated

4 years ago
Status: RESOLVED → VERIFIED
This issue STILL OCCURS on the Flame v2.1 and v2.2. Same issue as bug 1081562.

Environmental Variables:

Device: Flame 2.1 KK (319 MB)
BuildID: 20141011000201 (full flash)
Gaia: f5d4ff60ffed8961f7d0380ada9d0facfdfd56b1
Gecko: d813d79d3eae
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame 2.2 Master KK (319 MB)
BuildID: 20141011040204 (full flash)
Gaia: 95f580a1522ffd0f09302372b78200dab9b6f322
Gecko: 3f6a51950eb5
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 touch area for the share options in the camera are too low.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Whiteboard: [failed-verification]
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+] [failed-verification]
Flags: needinfo?(ktucker)
Whiteboard: [failed-verification]
Status: VERIFIED → RESOLVED
Last Resolved: 4 years ago4 years ago
(Assignee)

Updated

4 years ago
Flags: in-testsuite-

Comment 26

4 years ago
Verified the issue is fixed on 2.2 and 2.1 Flame devices
Ellipses button is quickly response on the first touch

Flame 2.2 KK (319mb Full Flash)
Device: Flame 2.2 Master
BuildID: 20141022040201
Gaia: 27a1d1baaa8e375b70e043efee67d5f2206c330b
Gecko: ae4d9b4ff2ee
Gonk: 05aa7b98d3f891b334031dc710d48d0d6b82ec1d
Version: 36.0a1 (2.2 Master)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Flame 2.1
Device: Flame 2.1 KK (319mb Full Flash)
BuildID: 20141022001201
Gaia: 3d9cc667f4e929861a9a77c41096bbf5a9c1bde0
Gecko: 928b18f7d8ff
Gonk: 05aa7b98d3f891b334031dc710d48d0d6b82ec1d
Version: 34.0 (2.1)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] [failed-verification] → [QAnalyst-Triage?]
status-b2g-v2.1: fixed → verified
status-b2g-v2.2: fixed → verified
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.