Implement App Titlebar

RESOLVED FIXED in 2.1 S1 (1aug)

Status

Firefox OS
Gaia::System
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: vingtetun, Assigned: etienne)

Tracking

unspecified
2.1 S1 (1aug)
Dependency tree / graph

Firefox Tracking Flags

(feature-b2g:2.1)

Details

(Whiteboard: [systemsfe][tako])

Attachments

(4 attachments)

Here is the branch with the various patches. It still miss some tests.

https://github.com/etiennesegonzac/gaia/tree/titlebar
Blocks: 1038738
(In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) from comment #1)
> Here is the branch with the various patches. It still miss some tests.
> 
> https://github.com/etiennesegonzac/gaia/tree/titlebar

Hi Vivien and Etienne...

Francis and I reviewed the patch today as well as the video (http://sgz.fr/in-app-rocketbar.webm). We agree that this is the preferred approach to handling the edge swipe transition for the status bar as it gracefully handles colour matching, state changes and orientation changes. 

Adding visual design for further feedback and we'll be available during the working hours tomorrow if you have further questions.

Thanks!
Rob
Flags: needinfo?(pabratowski)
Flags: needinfo?(epang)
Hi, 

This looks good, it's the way I was expecting it to behave. I just have a few notes regarding visuals.

1.  The icon on the sheet overlay - The edges are very rough, is there anything we can do?
2.  Sheets overlay - Is it possible to quickly fade out the overlay, icon and app name?  See the animations here: https://mozilla.box.com/s/e2es02iaskszkb1xuj4t

Thanks!
Eric
Flags: needinfo?(epang)

Updated

3 years ago
Blocks: 1039519
Overall this looks great! 
I agree with Eric on his notes. One more thing to add, is that how dark the small version of rocker bar should be? Feels very dark to me when it's sitting there in status bar.
Flags: needinfo?(pabratowski)
Duplicate of this bug: 1041589
(Assignee)

Updated

3 years ago
Depends on: 1043272
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1041589
(Assignee)

Comment 7

3 years ago
Created attachment 8461430 [details] [review]
Gaia PR

Much easier to review :)
Assignee: nobody → etienne
Attachment #8461430 - Flags: review?(alive)
(Assignee)

Comment 8

3 years ago
Created attachment 8461437 [details] [diff] [review]
Turning theme colors on

This is a standalone PR but to actually see the results you need the theme color to be turned on.
Since the settings app is out of sync with the actual setting name you need to turn it on in build/ for testing.


But as you can see we'll land the theme-color turned on by default before we land this patch.
Comment on attachment 8461430 [details] [review]
Gaia PR

The standalone one looks sane to me.
Attachment #8461430 - Flags: review?(alive) → review+
(Assignee)

Comment 10

3 years ago
Created attachment 8461626 [details]
Title bar for app with a theme-color
Attachment #8461626 - Flags: ui-review?(fdjabri)
(Assignee)

Comment 11

3 years ago
Created attachment 8461628 [details]
Title bar for app without a theme-color
Attachment #8461628 - Flags: ui-review?(fdjabri)
(Assignee)

Comment 12

3 years ago
Marionette tests fixed (thank god for marionette tests), a new try-build is in progress.
(Assignee)

Comment 13

3 years ago
Comment on attachment 8461628 [details]
Title bar for app without a theme-color

Clearing.

I can't find a final VsD (and I know some of the color details are still pending), and we'll need to finish the status bar icons priority before we can do the tweaks to get to pixel perfect.

Since this is blocking other work I'm going to land it as is.
Attachment #8461628 - Flags: ui-review?(fdjabri)
(Assignee)

Updated

3 years ago
Attachment #8461626 - Flags: ui-review?(fdjabri)
(Assignee)

Comment 14

3 years ago
https://github.com/mozilla-b2g/gaia/commit/9f4ad7301db7b3a034f012f0af4eb97df06c818e
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Depends on: 1043599
(Assignee)

Comment 15

3 years ago
This is causing a launch time performance regression, I have a fix, will file a follow up and send the patch ASAP.
(Assignee)

Updated

3 years ago
Blocks: 1043857
feature-b2g: --- → 2.1
Whiteboard: [systemsfe][tako]
Target Milestone: --- → 2.1 S1 (1aug)
This bug caused a serious regression of statusbar accessibility: bug 1045017. In particular, the use of -moz-element(#statusbar) together with moving the statusbar container offscreen basically renders statusbar useless to a screen reader user.
Depends on: 1045017
(In reply to Yura Zenevich [:yzen] from comment #16)
> This bug caused a serious regression of statusbar accessibility: bug
> 1045017. In particular, the use of -moz-element(#statusbar) together with
> moving the statusbar container offscreen basically renders statusbar useless
> to a screen reader user.

What options do we have here? Is there any platform plans to improve accessibility of moz-element? Can we proxy or shift focus to another element perhaps?
(In reply to Kevin Grandon :kgrandon from comment #17)
> (In reply to Yura Zenevich [:yzen] from comment #16)
> > This bug caused a serious regression of statusbar accessibility: bug
> > 1045017. In particular, the use of -moz-element(#statusbar) together with
> > moving the statusbar container offscreen basically renders statusbar useless
> > to a screen reader user.
> 
> What options do we have here? Is there any platform plans to improve
> accessibility of moz-element? Can we proxy or shift focus to another element
> perhaps?

I have a feeling that the bg image should not have an a11y API support based on what it is. I would suggest not using moz-element at all, because it is and will be (mis)used for purposes that it was not intended to. I wonder it would be possible to implement the set feature using the global status bar? We should definitely discuss this further, but in the meantime I would vote to back this change out.
I agree with Yura, STRONGLY.

What this moz-element thingie does is turn all the good semantic stuff that is in the HTML of the status bar and its icons into a blackbox that is even less accessible than Canvas. Even worse, the elements have to stay in the DOM anyway, but their bounds are now completely bogus.

I don't think -moz-element was ever intended to take whole stacks of interactive elements and stack them away in a background image like that.

This is a big STOP sign to this approach, it must be backed out and reimplemented in a *proper* semantic way. There is no other way to make this accessible.
So I think we need to consider all options before an immediate backout. We still have lots of time in 2.1, but backing this out would put the release at risk, and prevent us from making our commitments to our partners. Some questions:

1 - If the  platform can not be smarter when the screen reader encounters an element with moz-element, could we add some type of aria-attribute to proxy focus to the other element?

2 - Is there some way to manually control focus and shift it to the other statusbar?

3 - How do we feel about changing functionality if the screen reader is enabled? One option might be to just to restore the old statusbar when the screen reader is enabled. The main reason for this patch is to support background colors and visual transitions of the rocketbar. These may be less important to have if the screen reader is enabled.
Flags: needinfo?(yzenevich)
Flags: needinfo?(marco.zehe)
(In reply to Kevin Grandon :kgrandon from comment #20)
> So I think we need to consider all options before an immediate backout. We
> still have lots of time in 2.1, but backing this out would put the release
> at risk, and prevent us from making our commitments to our partners. Some
> questions:

I'm not suggesting we back this out as a feature, but as the implementation and reimplement it properly.

> 1 - If the  platform can not be smarter when the screen reader encounters an
> element with moz-element, could we add some type of aria-attribute to proxy
> focus to the other element?

Focus is not the problem. The lack of representation is. The elements can be swiped to, they just can no longer be acted on. And they cannot be found via explore by touch. They are still in the DOM, but because they are basically put into that black box, they are elements without relation to the screen. The screen itself has an area with no relation to the elements in the DOM.

Having said that, there is no ARIA attribute to handle a case like this.

> 2 - Is there some way to manually control focus and shift it to the other
> statusbar?

This is already happening when swiping sequentially (see above).

> 3 - How do we feel about changing functionality if the screen reader is
> enabled? One option might be to just to restore the old statusbar when the
> screen reader is enabled. The main reason for this patch is to support
> background colors and visual transitions of the rocketbar. These may be less
> important to have if the screen reader is enabled.

Badly, because separate code paths always are prone to getting out of sync and thus increasing dev cost.

And by the way, I tweeted earlier how easy it is to do horrible things with some CSS functions, and this is a reply I got from Chris Heilmann, and which I think should be reason enough to very seriously reconsider implementation:
https://twitter.com/codepo8/status/494159651173597184

Do you really want to base such an important implementation on something that is considered more or less deprecated and a security risk?
Flags: needinfo?(marco.zehe)
(In reply to Marco Zehe (:MarcoZ) from comment #19)
> I agree with Yura, STRONGLY.
> 
> What this moz-element thingie does is turn all the good semantic stuff that
> is in the HTML of the status bar and its icons into a blackbox that is even
> less accessible than Canvas. Even worse, the elements have to stay in the
> DOM anyway, but their bounds are now completely bogus.
> 
> I don't think -moz-element was ever intended to take whole stacks of
> interactive elements and stack them away in a background image like that.
> 
> This is a big STOP sign to this approach, it must be backed out and
> reimplemented in a *proper* semantic way. There is no other way to make this
> accessible.

There are pros and cons about using moz-element. In this case we want to maintain a single statusbar state, while displaying one statusbar per app window. Mostly to be edge gestures friendly.

Does edge gesture works with a screen reader ?
(In reply to Marco Zehe (:MarcoZ) from comment #21)
> (In reply to Kevin Grandon :kgrandon from comment #20)
> > So I think we need to consider all options before an immediate backout. We
> > still have lots of time in 2.1, but backing this out would put the release
> > at risk, and prevent us from making our commitments to our partners. Some
> > questions:
> 
> I'm not suggesting we back this out as a feature, but as the implementation
> and reimplement it properly.
> 

Let's try to understand the situation, and what works and not for accessibility (edge gestures?) that has led to the usage of -moz-element.
(In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) from comment #22)
> (In reply to Marco Zehe (:MarcoZ) from comment #19)
> > I agree with Yura, STRONGLY.
> > 
> > What this moz-element thingie does is turn all the good semantic stuff that
> > is in the HTML of the status bar and its icons into a blackbox that is even
> > less accessible than Canvas. Even worse, the elements have to stay in the
> > DOM anyway, but their bounds are now completely bogus.
> > 
> > I don't think -moz-element was ever intended to take whole stacks of
> > interactive elements and stack them away in a background image like that.
> > 
> > This is a big STOP sign to this approach, it must be backed out and
> > reimplemented in a *proper* semantic way. There is no other way to make this
> > accessible.
> 
> There are pros and cons about using moz-element. In this case we want to
> maintain a single statusbar state, while displaying one statusbar per app
> window. Mostly to be edge gestures friendly.
> 
> Does edge gesture works with a screen reader ?

The edge-gestures are not supported by the screen reader ATM since the screen reader needs to be focused on something to be actionable.
(In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) from comment #23)
> (In reply to Marco Zehe (:MarcoZ) from comment #21)
> > (In reply to Kevin Grandon :kgrandon from comment #20)
> > > So I think we need to consider all options before an immediate backout. We
> > > still have lots of time in 2.1, but backing this out would put the release
> > > at risk, and prevent us from making our commitments to our partners. Some
> > > questions:
> > 
> > I'm not suggesting we back this out as a feature, but as the implementation
> > and reimplement it properly.
> > 
> 
> Let's try to understand the situation, and what works and not for
> accessibility (edge gestures?) that has led to the usage of -moz-element.

This a11y regression is not related to the edge gestures. The issue is with status bar icons accessibility that is now lost since they are presented as a background image for the app titlebar; also with the off-screen content (which is where the real statusbar is).
Flags: needinfo?(yzenevich)
(In reply to Marco Zehe (:MarcoZ) from comment #21)
> 
> > 1 - If the  platform can not be smarter when the screen reader encounters an
> > element with moz-element, could we add some type of aria-attribute to proxy
> > focus to the other element?
> 
> Focus is not the problem. The lack of representation is. The elements can be
> swiped to, they just can no longer be acted on. And they cannot be found via
> explore by touch. They are still in the DOM, but because they are basically
> put into that black box, they are elements without relation to the screen.
> The screen itself has an area with no relation to the elements in the DOM.
> 
> Having said that, there is no ARIA attribute to handle a case like this.

Is there an ARIA attribute that we could standardize and create for this? Who should we talk to in order to discuss this?
(In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) from comment #22)
> Does edge gesture works with a screen reader ?

They are part of what's now broken. Before this, one could touch one of the status bar icons, then do a two-finger swipe down to bring up the utility tray. (this is with the screen reader on of course). One could also swipe left and right through the items sequentially, which would visually shift the screen reader focus from one item to the next.

When this landed, the ability to touch the status bar items was lost. It's now like a white sheet of paper there, nothing there. One can still swipe to the status bar items in the DOM, but these are the ones now off-screen and no longer reacting to the two-finger swipe down. But that is all. The two-finger swipe down does no longer work, and since there is nothing else up there in the status bar to screen-reader-focus, there is nothing to perform the two-finger swipe down on.
(In reply to Kevin Grandon :kgrandon from comment #26)
> Is there an ARIA attribute that we could standardize and create for this?
> Who should we talk to in order to discuss this?

Me, and Davidb.

But before we do that, we *must* explore all other alternatives. Besides, you didn't answer my last question re the deprecation of -moz-element. ARIA must be always the last possible solution, not the first.
(In reply to Marco Zehe (:MarcoZ) from comment #27)
> (In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails,
> needinfo? please) from comment #22)
> > Does edge gesture works with a screen reader ?
> 
> They are part of what's now broken. Before this, one could touch one of the
> status bar icons, then do a two-finger swipe down to bring up the utility
> tray. (this is with the screen reader on of course). One could also swipe
> left and right through the items sequentially, which would visually shift
> the screen reader focus from one item to the next.

Sorry, I meant edge gestures from the side of the device. You should normally be able to swipe from app to app by using the left/right edge of the device.

 
> When this landed, the ability to touch the status bar items was lost. It's
> now like a white sheet of paper there, nothing there. One can still swipe to
> the status bar items in the DOM, but these are the ones now off-screen and
> no longer reacting to the two-finger swipe down. But that is all. The
> two-finger swipe down does no longer work, and since there is nothing else
> up there in the status bar to screen-reader-focus, there is nothing to
> perform the two-finger swipe down on.

Also how do you use to bring the statusbar in view in fullscreen mode ? (it was already hidden by default in this case).
The edge gestures from left and right aren't supported by the screen reader yet. I am not sure about full-screen, whenever I use thed the device, from before this breaking patch, I always had access to the status bar. So I guess I never found out how full-screen for an app is invoked, or this has also not been supported by the screen reader yet.

Yura, have you ever tried this?
Flags: needinfo?(yzenevich)
(In reply to Marco Zehe (:MarcoZ) from comment #28)
> But before we do that, we *must* explore all other alternatives. Besides,
> you didn't answer my last question re the deprecation of -moz-element. ARIA
> must be always the last possible solution, not the first.

I think we've found some valid use-cases for moz-element here, so I would argue against the deprecation - maybe it's something we should support and provide ways to make it accessible?
(In reply to Marco Zehe (:MarcoZ) from comment #30)
> The edge gestures from left and right aren't supported by the screen reader
> yet. I am not sure about full-screen, whenever I use thed the device, from
> before this breaking patch, I always had access to the status bar. So I
> guess I never found out how full-screen for an app is invoked, or this has
> also not been supported by the screen reader yet.
> 
> Yura, have you ever tried this?

Prior to this PR, if the statusbar was hidden it would not be navigable by the screen reader (we explicitly set its visibility, not just transition off screen). In other words, there was no way to get to the status bar when it was hidden.
Flags: needinfo?(yzenevich)
Sorry to disappoint, but CSS background images are bad for many accessibility reasons. On desktops, they disappear as soon as the operating system's high-contrast mode is used, for example, at least on Windows. When we introduce such a feature into Firefox OS, which we most likely will at some point, we may very well be affected by problems resulting from heavy CSS background image use as well.

Zooming also has issues, and this will affect FF OS, too, much earlier than the above high contrast or color inversion modes.

So while the path to accomplishing the desired effects my seem easy on first glance with -moz-element, the ramifications beyond the screen reader problem at hand are quite uncalculable.
(In reply to Kevin Grandon :kgrandon from comment #31)
> (In reply to Marco Zehe (:MarcoZ) from comment #28)
> > But before we do that, we *must* explore all other alternatives. Besides,
> > you didn't answer my last question re the deprecation of -moz-element. ARIA
> > must be always the last possible solution, not the first.
> 
> I think we've found some valid use-cases for moz-element here, so I would
> argue against the deprecation - maybe it's something we should support and
> provide ways to make it accessible?

I would not argue that there are valid use-cases for moz-element: reflections, transitions, etc (https://hacks.mozilla.org/2010/08/mozelement/). Most of the use cases mentioned in the examples, are for presentational purposes. IMO, the example with text as background image (without at least some textual alternative) is invalid.

Some of the concerns that need to be accounted for when moz-element is considered (IMO):
Does the markup that is transformed into the image carry significant semantic value?
Can the markup that is transformed into the image be considered atomic?
I could see how -moz-element works fine with a11y in some places, specifically when it is for optimizing transitions or providing app screenshots. Semantically it makes sense: "this is a card that represents a background app with a screenshot of a running instance".

As Marco and Yura already mentioned, the use here puts a11y in a tough spot. This is not going to be easy or even possible to program around it. We rely on the fact that gecko layout knows where certain items are and makes them discoverable with coordinate queries. I don't think any special attributes (standard aria or not) will help here.

I am skeptical of keeping diverging code paths for screen reader users. Besides the usual critiques, I just can't imagine that an alternative status bar will be kept current and properly maintained.
There are two options I see here:

1) Change the platform to have some way of moz-element focus surface the original content and make things accessible for us. Might need a new aria attribute for this.

2) Diverge and use a different statusbar, probably without theme color or rocketbar, when the screen reader is enabled.

I don't really think we will have much luck trying to put statusbar logic into every app window. The performance hit will be too great.
(In reply to Kevin Grandon :kgrandon from comment #36)
> There are two options I see here:
> 
> 1) Change the platform to have some way of moz-element focus surface the
> original content and make things accessible for us. Might need a new aria
> attribute for this.
> 
> 2) Diverge and use a different statusbar, probably without theme color or
> rocketbar, when the screen reader is enabled.
>

We can probably found a solution to place the original statusbar at the previous position when the screen reader is enabled. It is a bit z-indexes tricky to get everything right but that should be doable.
 
> I don't really think we will have much luck trying to put statusbar logic
> into every app window. The performance hit will be too great.

Performance / memory and states management will result into a lot of work.

Can we just open a followup and track the accessibility issue there ?
(In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) from comment #37)
> > I don't really think we will have much luck trying to put statusbar logic
> > into every app window. The performance hit will be too great.
> Performance / memory and states management will result into a lot of work.

I still don't see how stuffing it all into a background image helps here. If you do it, you still need the reference to all the bits anyway, so you already carry them around in the different apps.

> Can we just open a followup and track the accessibility issue there ?

Already done: Bug 1045017.
(In reply to Marco Zehe (:MarcoZ) from comment #38)
> (In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails,
> needinfo? please) from comment #37)
> > > I don't really think we will have much luck trying to put statusbar logic
> > > into every app window. The performance hit will be too great.
> > Performance / memory and states management will result into a lot of work.
> 
> I still don't see how stuffing it all into a background image helps here. If
> you do it, you still need the reference to all the bits anyway, so you
> already carry them around in the different apps.
> 

We can unload it the image pretty easily if the app if offscreen and not reachable directly by edge gestures.


> > Can we just open a followup and track the accessibility issue there ?
> 
> Already done: Bug 1045017.

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