1004542 - Back key remains selected when pressed in Music app to transition from PlayerView to SubListView

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Nivi]

STR:

1. Go to Music app->Playlist->Most played.
2. Click on a song. It starts playing.
3. Click on back button.
4. We go back to the Most played screen. But the back key is still selected.

Expected UI behavior: back key should not be selected when we go back to the Most played playlist screen.

Note: When I put the music app in the background or press the home key, the screen gets refreshed and back key is not selected anymore.

It seems like a simple screen transition issue.

Comment 1

[Jason Smith [:jsmith]]

Does this reproduce on a 1.3 device?

Comment 2

[Harvey]

This issue DOES reproduce on 1.3

1.3 Environmental Variables:
Device: Buri
BuildID: 20140501053808
Gaia: 667539f1ed4becc45b182a5f1046221d3eeb9e7c
Gecko: f3601c4c3d59
Version: 28.0
Firmware Version: v1.2-device.cfg

Comment 3

[Dominic Kuo [:dkuo]]

Nivi, would you please also attach the screenshot? I think it's probably caused by the visual/css changes, thanks.

Comment 4

[Nivi]

Attachment: On playlist screen initially

This is the initial screen.

Comment 5

[Nivi]

Attachment: In the player screen after clicking on a song.

Player mode - Music app. Playing the selected song.

Comment 6

[Nivi]

Attachment: Player list screen after pressing the back key.

Note that the back key is still selected when we go back to the Playlist screen.

Comment 7

[bhavana bajaj [:bajaj]]

Don't think this is a blocker for 1.4 but definitely a nice to have and improve our quality. :dkuo can you fix this for 2.0 ?

Comment 8

[Dominic Kuo [:dkuo]]

yes, and first I can see if it reproduces on master, it should be caused by css or gecko changes so will take some time to find the root cause, but probably already fixed since gecko is always changing.

Comment 9

[Hema Koka [:hema]]

Putting this in non-blocking but priority bucket.

Comment 10

[Hema Koka [:hema]]

We have a music app refresh (next major revision) that is being planned soon with features like playlists. 

Jim, If this is a simple fix, lets try to get the fix in for 2.0 (dominic suspects that it could be a gecko issue too). Also just a note that headers with back key navigation is possibly being updated as part of visual refresh (need to confirm for music app)

Comment 11

[Hema Koka [:hema]]

possibly related to: https://bugzilla.mozilla.org/show_bug.cgi?id=995532

Comment 12

[David Flanagan [:djf]]

Probably the same bug as 988086.  I don't think either one is a media bug, but I don't know who should be looking at it.  System team?

Comment 13

[Jim Porter (:squib)]

Yeah, I've been seeing this stuff all over Gaia (the settings app is another big one). I really think it's a Gecko bug.

Comment 14

[Dominic Kuo [:dkuo]]

Found a workable patch and I am working on it.

Comment 15

[Dominic Kuo [:dkuo]]

Attachment: Add :focus styles to the BB headers

Arnau,

I have found the back key remains selected/highlighted is because the :focus style, and looks like it inherits the :hover/:active(probably a gecko issue?) style so after we pressed the back button, it gets focused and :focus style will be applied. My patch is to add the :focus styles individually to all the headers then force the background-color to be same as the header's, but not sure if it's a right approach since it modified all the header's style in the building block, would you please comment on this? thanks.

Comment 16

[Andrew Overholt [:overholt]]

Recently there were some issues with :active but I think they're fixed now.  In https://bugzilla.mozilla.org/show_bug.cgi?id=995532#c15 I said:  "kats says bug 1013378 should fix most of the recent :active bustage (likely regressed by bug 964750)."

If things are still broken, I recommend getting kats and botond involved and perhaps moving the bug to Core::Panning and Zooming.

Comment 17

[bhavana bajaj [:bajaj]]

(In reply to Andrew Overholt [:overholt] from comment #16)
> Recently there were some issues with :active but I think they're fixed now. 
> In https://bugzilla.mozilla.org/show_bug.cgi?id=995532#c15 I said:  "kats
> says bug 1013378 should fix most of the recent :active bustage (likely
> regressed by bug 964750)."
> 
> If things are still broken, I recommend getting kats and botond involved and
> perhaps moving the bug to Core::Panning and Zooming.

Let me re-confirm with QA if this is reproducible still on 2.0, given the landing 1013378 a few days back which may have helped this issue as :overholt recommends.

Comment 18

[Arnau March [:arnau] ( not working in Firefox OS anymore :( )]

Dominic,
the :focus idea to block :hover effect works great!
Could you please replace the selected color in your patch with `transparent`?
This way we won't have to update it in case we are changing a header background-color.
We would also need that fix in web components.
Could you please add the focus state in shared/elements/gaia_header/style.css?
(every time you see a :hover)

Please add me as a reviewer when you update your patch :)

Comment 19

[Harvey]

Attempting to reproduce the issue using the STRs causes a crash on the latest Central Buri build upon playing a song in the Most played screen.

Environmental Variables:
Device: Buri
BuildID: 20140529075050
Gaia: b669dd2cc321f37cebc7081a79b968cac36b4200
Gecko: a3848fbadb16
Version: 32.0a1
Firmware Version: v1.2-device.cfg

Crash report: https://crash-stats.mozilla.com/report/index/ee7c4f30-ba55-4744-bb4f-65f222140529

Comment 20

[Jason Smith [:jsmith]]

Can we try testing this on Flame on 2.0? There's no crash symbols in that report...

Comment 21

[Dominic Kuo [:dkuo]]

Comment on attachment 8430117 [details] [review]
Add :focus styles to the BB headers

Arnau, I have changed the :focus background-color to transparent, also added the styles to the web component headers, would you please take a look on the patch again? thanks.

Comment 22

[Arnau March [:arnau] ( not working in Firefox OS anymore :( )]

Comment on attachment 8430117 [details] [review]
Add :focus styles to the BB headers

Works Great! Thanks Dominic.
Please squash your commits before landing.

Comment 23

[Joshua Mitchell (Inactive)]

(In reply to Jason Smith [:jsmith] from comment #20)
> Can we try testing this on Flame on 2.0? There's no crash symbols in that
> report...

This issue DOES still reproduce on the latest flame 2.0
2.0 Environmental Variables:
Device: Flame 2.0
BuildID: 20140604064615
Gaia: a38a6a5c6fabc97dd16d5360632b5ac5c7e06241
Gecko: c7fdd7e755cd
Version: 32.0a1
Firmware Version: v10G-2

Comment 24

[Dominic Kuo [:dkuo]]

It might be the gecko issue that Andrew mentioned in comment 16, but since the gaia patch won't harm the headers and added the :focus styles(which is better and safer for the buttons when they are focused), so I am going to land it without waiting/finding the gecko fix.

Comment 25

[Dominic Kuo [:dkuo]]

Landed on master: a98a7a3bb570c893c07afeab52546fd3f67e30bb