1014052 - [Status bar Accessibility] The swiping order of items is reversed

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Marco Zehe (:MarcoZ)]

The order of items in the status bar is reversed. When swiping left to right to advance, the status bar items are actually traversed right to left.

STR:
1. Turn on screen reader.
2. Touch the WiFi icon in the status bar.
3. Swipe to the right.

Expected: Rectangle moves to the battery icon.
Actual: Moves to the icon left to the WiFi symbol, in the case of the Flame the SIM 2 indicator.

Comment 1

[Yura Zenevich [:yzen] (please use "needinfo")]

This can probably be fixed with CSS flex.

Comment 2

[Eitan Isaacson [:eeejay]]

Re-aligning priorities with 2.1 accessibility goals.

Comment 3

[Manuel Casas Barrado [:mancas]]

Attachment: Reorder items in the status bar

Hi Yura, I've been looking for you in IRC channel because I want to fix this bug. Here you have my PR.

Thanks

Comment 4

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Manuel Casas Barrado [:mancas] from comment #3)
> Created attachment 8453615 [details] [review]
> Reorder items in the status bar
> 
> Hi Yura, I've been looking for you in IRC channel because I want to fix this
> bug. Here you have my PR.
> 
> Thanks

Hi Manuel, a good first pass. There are some issues that need to be addressed mainly because of the things that are not visible by default (the lefthandside notifications, etc that I mentioned in the pull request). Also see if you can use flex-direction to avoid restructuring markup.

Comment 5

[Manuel Casas Barrado [:mancas]]

(In reply to Yura Zenevich [:yzen] from comment #4)
> (In reply to Manuel Casas Barrado [:mancas] from comment #3)
> > Created attachment 8453615 [details] [review]
> > Reorder items in the status bar
> > 
> > Hi Yura, I've been looking for you in IRC channel because I want to fix this
> > bug. Here you have my PR.
> > 
> > Thanks
> 
> Hi Manuel, a good first pass. There are some issues that need to be
> addressed mainly because of the things that are not visible by default (the
> lefthandside notifications, etc that I mentioned in the pull request). Also
> see if you can use flex-direction to avoid restructuring markup.

Thanks for your comments. Please take a look at the new commit.

Comment 6

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Manuel Casas Barrado [:mancas] from comment #5)
> (In reply to Yura Zenevich [:yzen] from comment #4)
> > (In reply to Manuel Casas Barrado [:mancas] from comment #3)
> > > Created attachment 8453615 [details] [review]
> > > Reorder items in the status bar
> > > 
> > > Hi Yura, I've been looking for you in IRC channel because I want to fix this
> > > bug. Here you have my PR.
> > > 
> > > Thanks
> > 
> > Hi Manuel, a good first pass. There are some issues that need to be
> > addressed mainly because of the things that are not visible by default (the
> > lefthandside notifications, etc that I mentioned in the pull request). Also
> > see if you can use flex-direction to avoid restructuring markup.
> 
> Thanks for your comments. Please take a look at the new commit.

Hi Manuel, your pull request needs rebasing as it contains external commits.

Comment 7

[Yura Zenevich [:yzen] (please use "needinfo")]

Also, Manuel, you will need an actual review from the Systems app peer/owner, mark me as a11y-review?. Thanks.

Comment 8

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8453615 [details] [review]
Reorder items in the status bar

Thanks for the patch. a11y-r=me with just one fix (comment in Github).

Comment 9

[Etienne Segonzac (:etienne)]

Comment on attachment 8453615 [details] [review]
Reorder items in the status bar

Hey Mike, I'm forwarding this to you for the next round because I know you'll be spending a lot of time with this code during the next 2 weeks :)

Looks good overall but there's at least one issue with the notification count: when you drag down the utility tray the notification count should disappear, leaving room for the date but it's not, so the date is overlapping.

Also the notification count should be completely to the right in rtl languages, but it's not the case here (and we can't regress rtl).

Comment 10

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8453615 [details] [review]
Reorder items in the status bar

Hi Manuel,

Thank you again with your continued work on this! Please address the utility tray and RTL issues mentioned by Etienne, and also :yzen had one small comment on the PR. Flag me again when you have updated your commit :)

Comment 11

[Yura Zenevich [:yzen] (please use "needinfo")]

Hi Manuel, are you still working on this bug by any chance?

Comment 12

[Manuel Casas Barrado [:mancas]]

Hi Yura, I've been working in blockers bugs (2.0) so I have not much time to take care of this bug. However I've taken a look at the new status bar and there is something that it seems to be wrong. The status bar is cloned so in the DOM we have duplicate id attributes, is it a good practice? I don't think so

Comment 13

[Michael Henretty [:mikehenrty][:mhenretty]]

(In reply to Manuel Casas Barrado [:mancas] from comment #12)
> Hi Yura, I've been working in blockers bugs (2.0) so I have not much time to
> take care of this bug. However I've taken a look at the new status bar and
> there is something that it seems to be wrong. The status bar is cloned so in
> the DOM we have duplicate id attributes, is it a good practice? I don't
> think so

Are you referring to bug 1045017? Or are there other implications to worry about?

Comment 14

[Manuel Casas Barrado [:mancas]]

It seems the duplicate status bar was added in this merge https://github.com/mozilla-b2g/gaia/commit/7e4678128cdd6d296b67679fbe827845d19e7ec7#diff-293e71c28c7a1b418148d8af71d58804

Comment 15

[Yura Zenevich [:yzen] (please use "needinfo")]

(In reply to Manuel Casas Barrado [:mancas] from comment #14)
> It seems the duplicate status bar was added in this merge
> https://github.com/mozilla-b2g/gaia/commit/
> 7e4678128cdd6d296b67679fbe827845d19e7ec7#diff-
> 293e71c28c7a1b418148d8af71d58804

AFAIK the statusbar is duplicated as a banding image only using moz-element. The DOM elements are not.

Comment 16

[Yura Zenevich [:yzen] (please use "needinfo")]

s/banding/background

Comment 17

[Yura Zenevich [:yzen] (please use "needinfo")]

Attachment: Github pull request.

Comment 18

[Yura Zenevich [:yzen] (please use "needinfo")]

Hi Michael, do you think you'll have some time to take a look at the PR. It should be a little simpler now than the original one.

Comment 19

[Michael Henretty [:mikehenrty][:mhenretty]]

Hi Yura,

Yes, sorry it has taken me so long to review this. I will get to it either later today or tomorrow. Thanks for the patch!

Comment 20

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8481579 [details] [review]
Github pull request.

Wow, you weren't kidding this is simpler. I left a small comment on github, other than that this looks good to go.

Also, please make sure the tests are green before landing.

Comment 21

[Yura Zenevich [:yzen] (please use "needinfo")]

https://github.com/mozilla-b2g/gaia/commit/944cf2bb2a05869441d03763a7559f9d3efd8937

Comment 22

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8481579 [details] [review]
Github pull request.

This bug improves the way status is presented to the screen reader user. It makes the markup consistent with presentation on the screen instead of being reverse.

NOTE: depends on the approval of bug 1045017

[Bug caused by] (feature/regressing bug #): improvement, not a regression

[User impact] if declined: Status bar navigation will remain confusing for the screen reader user. When navigating forward or back the user will actually be moved in the opposite direction (back and forward respectively).

[Testing completed]: No JS changes just markup and CSS, manual testing didn't identify any problems.

[Risk to taking this patch] (and alternatives if risky): Fairly low risk. Only statusbar HTML and CSS changed.

[String changes made]: None

Comment 23

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/61a5cf57a56843f1340bd0f6b5f985b55de641f9

Comment 24

[Coler]

This issue has been verified successfully on lame2.1&2.2.
Reproducing rate: 0/5
See attachment: Verify_Flame_Swiporder.mp4

Flame2.1 build version:
Gaia-Rev        ccb49abe412c978a4045f0c75abff534372716c4
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/18fb67530b22
Build-ID        20141130001203
Version         34.0

Flame2.2 build version:
Gaia-Rev        7119da7a86cd803840678ca3a6067e5622adc481
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/df3fc7cb7e80
Build-ID        20141130040207
Version         37.0a1

Comment 25

[Coler]

Attachment: Verify_Flame_Swiporder.MP4