1160344 - [Gallery] Dotted lines appear when tapping a video file

Status: VERIFIED FIXED

Description

[Yeojin Chung [:YeojinC]]

Created attachment 8600113 [details]
logcat_dottedLines.txt

Description:
When the user taps the video file in Gallery app, dotted lines appear at the top and bottom of the frame.

Pre-requisite: Have at least 1 video file.

Repro Steps:
1) Update a Flame to 20150430010201.
2) Open Gallery app.
3) Select a video file. 
4) Press the play button.
5) Tap anywhere within the video frame.

Actual:
Dotted lines appear at the top and bottom of the screen.

Expected:
The controller disappears, and dotted lines do not appear around the frame.

Environmental Variables:
Device: Flame Master (KK, 319mb, full flash)
Build ID: 20150430010201
Gaia: db8ea705c0fd1b1684807f5a8e837bb9a36a6f96
Gecko: 4b9b12c248dc
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (Master)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Repro frequency: 5/5
See attached: Video, logcat, screenshot
https://youtu.be/_HJMH3CMXlI

Comment 1

[Yeojin Chung [:YeojinC]]

This issue does NOT appear on Flame 2.2.

Result: Dotted lines do not appear when the frame is selected.

Environmental Variables:
Device: Flame 2.2 (KK, 319mb, full flash)
Build ID: 20150430002504
Gaia: aa1da5036f9425c25d515d14243d3473bfefb4fd
Gecko: 38b2838d43e1
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 2

[Yeojin Chung [:YeojinC]]

Created attachment 8600116 [details]
VideoFrameSelected.png

Comment 3

[KTucker [:KTucker][Inactive 3/4/2016]]

No-Jun, can you take a look at this please and weigh in on the severity of this regression?

Comment 4

[No-Jun Park [:njpark]]

I think this is an appearance issue so not worth blocking, but it's odd that it started to appear just now.  Perhaps a fallback from recent patch.  ni?ing djf for assessment.

Comment 5

[Yeojin Chung [:YeojinC]]

b2g-inbound Regression Window:

Last Working Environmental Variables:
Device: Flame 3.0
BuildID: 20150405145759
Gaia: 723a80af3e8ce11bdea46685f3f68b934ed1aa0c
Gecko: 11fab330738e
Version: 40.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

First Broken Environmental Variables:
Device: Flame 3.0
BuildID: 20150406080758
Gaia: 9d9d41e22d578105c9285809413dc5c2953d900d
Gecko: d22a8bcd2890
Version: 40.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Last Working Gaia First Broken Gecko: Issue does NOT reproduce 
Gaia: 723a80af3e8ce11bdea46685f3f68b934ed1aa0c
Gecko: d22a8bcd2890

First Broken Gaia Last Working Gecko: Issue DOES reproduce
Gaia: 9d9d41e22d578105c9285809413dc5c2953d900d
Gecko: 11fab330738e

https://github.com/mozilla-b2g/gaia/compare/723a80af3e8ce11bdea46685f3f68b934ed1aa0c...9d9d41e22d578105c9285809413dc5c2953d900d


Caused by bug 1068912

Comment 6

[KTucker [:KTucker][Inactive 3/4/2016]]

Yura, can you take a look at this please? This looks to have been caused by the landing for bug 1068912.

Comment 7

[GitHub Autolander Bot]

Created attachment 8601038 [details] [review]
[gaia] yzen:bug-1160344 > mozilla-b2g:master

Comment 8

[David Flanagan [:djf]]

Comment on attachment 8601038 [details] [review]
[gaia] yzen:bug-1160344 > mozilla-b2g:master

Yura,

Instead of hiding the focus outline when the element is focused, wouldn't it be better to just prevent it from becoming focused when the user taps?  Can we just do a pointer-events:none on the video element instead? From an accessibility standpoint, isn't it in general a bad thing to hide focus rings?

Clearing the review flag and setting needinfo for this question.

If you have a reason for this hiding the focus outline approach instead of a prevent-focus approach, just explain why you are doing it that way and reset the r? flag.

Comment 9

[Yura Zenevich [:yzen]]

(In reply to David Flanagan [:djf] from comment #8)
> Comment on attachment 8601038 [details] [review]
> [gaia] yzen:bug-1160344 > mozilla-b2g:master
> 
> Yura,
> 
> Instead of hiding the focus outline when the element is focused, wouldn't it
> be better to just prevent it from becoming focused when the user taps?  Can
> we just do a pointer-events:none on the video element instead? From an
> accessibility standpoint, isn't it in general a bad thing to hide focus
> rings?

Yes it is not desired for keyboard navigation. However, with the screen reader we have our own highlighting border that sits on currently focused element (note, screen reader focus is different from keyboard focus). I think in this case it is OK, especially because we are using custom controls for video playback and avoid focused styling in the first place.

As for pointers-event: none, it should generally be avoided. Pointer-events: none renders any element inaccessible to the screen reader, which, is not desirable, especially here, since we allow toggling controls by activating the video element when the video is played.

What do you think? 

> 
> Clearing the review flag and setting needinfo for this question.
> 
> If you have a reason for this hiding the focus outline approach instead of a
> prevent-focus approach, just explain why you are doing it that way and reset
> the r? flag.

Comment 10

[David Flanagan [:djf]]

Comment on attachment 8601038 [details] [review]
[gaia] yzen:bug-1160344 > mozilla-b2g:master

Thanks for the explanation. This looks good to me.  Thanks for remembering to fix it in both apps.

Comment 11

[GitHub Autolander Bot]

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

Comment 12

[Verson Xiong (Leave from Mozilla)]

Per Comment 11,this bug has been landed and fixed on master.
This issue is verified fixed on latest build of Flame 3.0,the STR is same as Comment 0.
Reproduce rate:0/5
Actual results:The controller disappears, and dotted lines do not appear around the frame
See verify3.0.mp4

Flame 3.0(Fixed):
Build ID               20150510010201
Gaia Revision          5b2a150f6f5d29bddfaac13fcbbf099376f2f275
Gaia Date              2015-05-09 12:34:41
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/77d92f6d7679
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150510.045933
Firmware Date          Sun May 10 04:59:44 EDT 2015
Bootloader             L1TC000118D0

Comment 13

[Verson Xiong (Leave from Mozilla)]

Created attachment 8603938 [details]
verify3.0.mp4