1088723 - Status bar visible while handling pick activity with Camera

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System::Window Mgmt, defect)

Description

[:gerard-majax]

Reproduced on current master, with Flame KK and Nexus S.

STR:
 0. Open SMS app
 1. Compose new message
 2. Tap attachment, select Photo/Camera

Expected:
 Taking picture, no statusbar visible.

Actual:
 Taking picture, but the statusbar is visible.

Comment 1

[Stephany Wilkes]

Believe status bar should be visible. Checking with Katie.

Comment 2

[:gerard-majax]

(In reply to Stephany Wilkes from comment #1)
> Believe status bar should be visible. Checking with Katie.

As far as I remember, we were hiding statusbar previously for this usecase. And Android does too. Having a statusbar there is quite misleading (not to mention that we have perf issues on this topic, I remember Vivien working on this for the utility tray).

Comment 3

[katieC]

The camera, when accessed via SMS, should follow the same behaviour as camera app itself. The status bar is not visible by default, but can be activated by dragging from the top edge.

Comment 4

[:gerard-majax]

(In reply to katieC from comment #3)
> The camera, when accessed via SMS, should follow the same behaviour as
> camera app itself. The status bar is not visible by default, but can be
> activated by dragging from the top edge.

So there's clearly a bug, since we see the statusbar always, not just when dragging it.

Comment 5

[Manuel Casas Barrado [:mancas]]

Attachment: No status bar - patch

This is a first approach to solve this bug. Please let me know any change we should implement to improve the solution.

Thanks!

Comment 6

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8513307 [details] [review]
No status bar - patch

Alive should review this, but I hardly thinks hardcoding the origin of Camera app is the right solution.

Comment 7

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8513307 [details] [review]
No status bar - patch

Bad.
If we want to implement this feature, what we should do is control the "statusbar shadow" part in activityWindow: if we are fullscreen activity judging from manifest, we may need to hide the shadow statusbar on init. Or even don't render it.
https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_window.js#L511

Comment 8

[Manuel Casas Barrado [:mancas]]

Alive, the decission about if the activity is fullscreen or not, gives priority to the parent app:

this._fullscreen = this.rearWindow ?
                   this.rearWindow.isFullScreen() :
                   this.manifest ?
                   !!this.manifest.fullscreen :
                   false;

What you said about the |statusbar-shadow|, AFAIK, doesn't hide the status bar. If we want to prioritize the fullscreen mode from the child activity, we should change the order of the previous code. Could you clarify this, please?

Comment 9

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Manuel Casas Barrado [:mancas] from comment #8)
> Alive, the decission about if the activity is fullscreen or not, gives
> priority to the parent app:
> 
> this._fullscreen = this.rearWindow ?
>                    this.rearWindow.isFullScreen() :
>                    this.manifest ?
>                    !!this.manifest.fullscreen :
>                    false;
> 
> What you said about the |statusbar-shadow|, AFAIK, doesn't hide the status
> bar. If we want to prioritize the fullscreen mode from the child activity,
> we should change the order of the previous code. Could you clarify this,
> please?

Apparent it is not designed to use the original fullscreen property because long time ago statusbar is not working with AppWindow well and we are not requested to do that. Now since you want to change the behavior, please fix that function.
Note: this is a feature request. We should be careful about any regressions.

Comment 10

[Kevin Grandon :kgrandon]

Adding a reference to bug 1090152, seems likely that the same patch could regress both of these.

Comment 11

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

Putting it in front of the polish team's eyes.

Comment 12

[Dietrich Ayala (:dietrich)]

Per comment #10, is this bug still relevant?

Comment 13

[:gerard-majax]

I still see the issue on my build that is a couple of days old ...

Comment 14

[Manuel Casas Barrado [:mancas]]

Comment on attachment 8513307 [details] [review]
No status bar - patch

Alive, I've modified the function that handle the fullscreen mode. Now, the function prioritizes the fullscreen mode of the activity that is going to be opened, but using the fullscreen mode of its parent app, if the fullscreen mode in the manifest is not defined.

Could you review it when you get a chance?
Thanks!

Comment 15

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8513307 [details] [review]
No status bar - patch

LGTM, thank you Mancas.

Comment 16

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

master: https://github.com/mozilla-b2g/gaia/commit/9534a664e50fefe5c7a9face4de61a2e11b95e8e

Comment 17

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

Manuel, please request v2.2 uplift when you get the chance.

Comment 18

[Manuel Casas Barrado [:mancas]]

Comment on attachment 8513307 [details] [review]
No status bar - patch

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Activity Window
[User impact] if declined: The status bar will be visible in fullscreen apps, camera per example, when it's opened from another app.
[Testing completed]: Yes
[Risk to taking this patch] (and alternatives if risky): Low
[String changes made]: No

Comment 19

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/20439b8528a21f61184f2f5173dad1bdffb40563