Closed
Bug 827708
Opened 11 years ago
Closed 11 years ago
[music] the seeking position is not always the same as the users tapping position
Categories
(Firefox OS Graveyard :: Gaia::Music, defect)
Tracking
(blocking-basecamp:-, b2g18+ fixed)
RESOLVED
FIXED
blocking-basecamp | - |
People
(Reporter: dkuo, Assigned: evanxd)
Details
(Keywords: regression)
Attachments
(1 file, 1 obsolete file)
359 bytes,
text/html
|
dkuo
:
review+
vingtetun
:
approval-gaia-v1+
|
Details |
Try to seek the player in Music app, we will see sometimes the tapped position is not the same as the sought position.
Reporter | ||
Updated•11 years ago
|
blocking-basecamp: --- → ?
Assignee | ||
Comment 1•11 years ago
|
||
Hi Dominic, I know how this bug happen. Let me take this. Thanks. :)
Assignee | ||
Updated•11 years ago
|
Assignee: dkuo → evanxd
Reporter | ||
Comment 2•11 years ago
|
||
It's easy to reproduce, about 50 pixel between the tap and seek position. and if users seek too close to the end of a song, it will be very easy to trigger the player to play the next song, which might not a expected result for users, because the users might just want to seek, not skip to next song.
Comment 3•11 years ago
|
||
Could we check if it is related to codec format ? Could you check if it's still reproducible ?
blocking-basecamp: ? → -
Keywords: qawanted
Reporter | ||
Comment 4•11 years ago
|
||
Hi David, it's not related to the codec format. Please see http://people.mozilla.com/~dkuo/B2G/Gaia/TestMedia/13010008.mp4 You can see the position of the seek indicator is not the same as the user's tapping position. It will jump to a totally different position.
Reporter | ||
Comment 5•11 years ago
|
||
I forgot to mention this is a regression since we have some changes of the seek bar. And it only need to change one line in CSS to fix this issue, so I am re-nom it again, thanks.
Keywords: regression
Reporter | ||
Updated•11 years ago
|
blocking-basecamp: - → ?
Comment 6•11 years ago
|
||
Tested with Build ID 20130107105806. It's not about codec and always reproducible. This is UX related. The seeker's position is a little bit right than where I hit. Evan should be able to fix the problem.
Comment 7•11 years ago
|
||
I may have broken this with the changes in bug 823619. The music player was using the non-standard event layerX property to get mouse coordinates relative to the slider region itself. I had to switch to clientX (or screenX, I forget which). I thought that I accounted for the offset, but maybe I didn't. It might be necessary to add a getClientBoundingRect() call or somethign to find the boundaries of the seekbar and map the event coordinates within that area.
Comment 8•11 years ago
|
||
Feel free to reassign to me if you get stuck on it, since it was probably me who broke it!
Comment 9•11 years ago
|
||
We would take a patch for this but we will not block V1.
blocking-basecamp: ? → -
tracking-b2g18:
--- → +
Reporter | ||
Comment 10•11 years ago
|
||
djf, It might be the changes recently that broken the slider, but I have checked the changes you made, the logic should be fine. The only thing that we didn't notice is, the target that listened to the mouse events is not always #player-seek-bar, it can also be #player-seek-bar-progress, so target.offsetLeft will be a different value, and after the calculation, the final result is incorrect and caused this issue. So the solution should be simple, we can just add "pointer-events: none" to #player-seek-bar-progress, and the target should be #player-seek-bar only. Evan Tseng is our newbie in Taipei office and I think he already got a patch for this, so it should be a good chance for him to practice how to solve a bug in bugzilla, thanks for pointing this out.
Assignee | ||
Comment 11•11 years ago
|
||
NOTE: If blocking-basecamp+ is set, just land it for now. [Approval Request Comment] Bug caused by (feature/regressing bug #): User impact if declined: Testing completed: Risk to taking this patch (and alternatives if risky):
Attachment #699562 -
Flags: review?(dkuo)
Attachment #699562 -
Flags: approval-gaia-master?(21)
Assignee | ||
Comment 12•11 years ago
|
||
NOTE: If blocking-basecamp+ is set, just land it for now. [Approval Request Comment] Bug caused by (feature/regressing bug #): User impact if declined: Testing completed: Risk to taking this patch (and alternatives if risky):
Attachment #699563 -
Flags: review?(dkuo)
Attachment #699563 -
Flags: approval-gaia-master?(21)
Assignee | ||
Updated•11 years ago
|
Attachment #699562 -
Flags: review?(dkuo)
Attachment #699562 -
Flags: approval-gaia-master?(21)
Assignee | ||
Updated•11 years ago
|
Attachment #699562 -
Attachment is obsolete: true
Reporter | ||
Comment 13•11 years ago
|
||
Comment on attachment 699563 [details] Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/7410 Looks good to me, r+.
Attachment #699563 -
Flags: review?(dkuo) → review+
Comment 14•11 years ago
|
||
Comment on attachment 699563 [details] Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/7410 CSS one liner. Sounds safe to me.
Attachment #699563 -
Flags: approval-gaia-master?(21) → approval-gaia-master+
Comment 15•11 years ago
|
||
https://github.com/mozilla-b2g/gaia/commit/1aaff974e8700857bc2494320f7252a919b36d0b
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
status-b2g18:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•