Last Comment Bug 772502 - Image slider remains highlighted after a tap is performed over it
: Image slider remains highlighted after a tap is performed over it
Status: VERIFIED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: Trunk
: ARM Android
: -- normal (vote)
: Firefox 16
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-10 09:15 PDT by Cristian Nicolae (:xti)
Modified: 2016-07-29 14:26 PDT (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
fixed
verified
verified


Attachments
screenshot 1 (148.14 KB, image/png)
2012-07-10 09:15 PDT, Cristian Nicolae (:xti)
no flags Details
screenshot 2 (171.73 KB, image/png)
2012-07-10 09:16 PDT, Cristian Nicolae (:xti)
no flags Details
Patch (1.05 KB, patch)
2012-07-10 11:24 PDT, Wesley Johnston (:wesj)
mark.finkle: review+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Cristian Nicolae (:xti) 2012-07-10 09:15:50 PDT
Created attachment 640629 [details]
screenshot 1

Firefox 16.0a1 (2012-07-10)
Device: Galaxy Nexus
OS: Android 4.0.4

Steps to reproduce:
1. Open Fennec
2. Browse to https://www.orange.ro/magazin-online/telefoane/sony-xperia-u#
3. Under the main pic on the left, there are 2 little thumbnails. Tap on the first one
4. Tap on the right side of the page

Expected result:
After step 4, the image is displayed correctly, without any slider highlights.

Actual result:
After step 4, the highlight border of the slider link is still visible (see screenshot 1). There is no way to make it disappear.

Note:
On the tablet stock browser, there is no slider highlight at all, as it happens for Native for example (see screenshot 2).
Comment 1 Cristian Nicolae (:xti) 2012-07-10 09:16:14 PDT
Created attachment 640630 [details]
screenshot 2
Comment 2 Wesley Johnston (:wesj) 2012-07-10 11:24:41 PDT
Created attachment 640694 [details] [diff] [review]
Patch

I hate our focus rings. This kills them. It makes me nervous as well.

My main concern is that I have seen cases where focus rings have worked and our tap highlighting has (for some reason) not worked. But I think I'd rather just find those and fix tap highlighting than keep these around.
Comment 3 Mark Finkle (:mfinkle) (use needinfo?) 2012-07-10 11:49:52 PDT
Comment on attachment 640694 [details] [diff] [review]
Patch

Let's test this out a bit on Nightly
Comment 5 Ryan VanderMeulen [:RyanVM] 2012-07-10 20:32:42 PDT
https://hg.mozilla.org/mozilla-central/rev/1e6251d8d492
Comment 6 Cristian Nicolae (:xti) 2012-07-11 23:52:52 PDT
This issue was fixed on the latest Nightly build. 
Closing bug as verified fixed on:

Firefox 16.0a1 (2012-07-11)
Device: Galaxy Nexus
OS: Android 4.0.4
Comment 7 Wesley Johnston (:wesj) 2012-07-24 09:27:43 PDT
Comment on attachment 640694 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Forever. Maemo Fennec 1.0 used this for tap highlighting I think.
User impact if declined: Orange rings around objects that shouldn't be there
Testing completed (on m-c, etc.): This has been on mc for two weeks and I haven't seen any complaints. Its a nice visual improvement in a many places.
Risk to taking this patch (and alternatives if risky): Low risk code wise. We use other methods to show what linked is being tapped and apparently they work well enough.
String or UUID changes made by this patch: None.
Comment 8 Lukas Blakk [:lsblakk] use ?needinfo 2012-07-24 14:37:55 PDT
This is already verified fixed on 16 (which is Aurora) -- are you intending to nominate this for Beta (15) and is 15 even affected?
Comment 9 Wesley Johnston (:wesj) 2012-07-24 14:38:51 PDT
Yes and yes. Sorry 'bout that. We've been shipping with this style of tap highlight for a long time.
Comment 10 Wesley Johnston (:wesj) 2012-07-24 14:39:05 PDT
Comment on attachment 640694 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Forever. Maemo Fennec 1.0 used this for tap highlighting I think.
User impact if declined: Orange rings around objects that shouldn't be there
Testing completed (on m-c, etc.): This has been on mc for two weeks and I haven't seen any complaints. Its a nice visual improvement in a many places.
Risk to taking this patch (and alternatives if risky): Low risk code wise. We use other methods to show what linked is being tapped and apparently they work well enough.
String or UUID changes made by this patch: None.
Comment 11 Lukas Blakk [:lsblakk] use ?needinfo 2012-07-24 15:06:10 PDT
Thanks for updating the status, I don't see that we need to track this since we've already shipped with it but I'll approve for beta landing since it's low risk and improves user experience with mobile.
Comment 12 Ryan VanderMeulen [:RyanVM] 2012-08-07 14:08:36 PDT
https://hg.mozilla.org/releases/mozilla-beta/rev/ad803799f66b

Note You need to log in before you can comment on or make changes to this bug.