1027990 - [System] ActivityWindow does not receive proper rearWindow

Status: RESOLVED FIXED

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

Description

[Kevin Grandon :kgrandon]

When diagnosing activities while the search app was running, I noticed that they do not receive the proper container element. In the search case this means that they are positioned behind the search app.

In order to implement bug 1027374 properly, we will need this in the long term. Looking for a workaround now.

Comment 1

[Kevin Grandon :kgrandon]

Alive, Aus - We have a temporary hack in place until we have a proper solution for activites in the search app here: https://github.com/mozilla-b2g/gaia/pull/20775/files#diff-96ad0e6b48f81d60d4bb14976f302924R60

Do you guys have any idea of the proper approach for this bug? Thanks!

Comment 2

[Aus Lacroix [:aus]]

Part of what would be required to get rid of the issue in bug 1027414 will probably benefit this bug as well. The ActivityWindow Manager should expose a way to determine if an AppWindow has an ActivityWindow that is associated with it and should be on top at all times. This will in turn be used in the AppWindowManager when Apps are requested to be displayed (as it should display the activity that was present before and not simply the application).

I think this may fix this issue. I'll keep you guys posted.

Comment 3

[Kevin Grandon :kgrandon]

Attachment: Probably a terrible fix

Hey Alive - any better ideas?

Comment 4

[Kevin Grandon :kgrandon]

Hey Aus - thanks for your comment, I didn't see that. I'm just looking at terrible workarounds right now, and I'm hoping there's a better way of doing things :)

Comment 5

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

(In reply to Kevin Grandon :kgrandon from comment #0)
> When diagnosing activities while the search app was running, I noticed that
> they do not receive the proper container element. In the search case this
> means that they are positioned behind the search app.

Place activity inside its caller is intentional, what's the real problem? Haven't read bug 1027374..

> 
> In order to implement bug 1027374 properly, we will need this in the long
> term. Looking for a workaround now.

Comment 6

[Kevin Grandon :kgrandon]

Comment on attachment 8443228 [details]
Probably a terrible fix

Uploading a new one

Comment 7

[Kevin Grandon :kgrandon]

Attachment: Hacky patch

Alive the problem is that the activity is getting the wrong caller when the searchWindow is used. Take a look at this line for what I had to do for the fix.

Comment 8

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

Comment on attachment 8443236 [details]
Hacky patch

I don't feel good about this fix and still don't know what's wanted.

If you want to render the activity called by searchWindow inside some certain appWindow, please redirect launchactivity request to the window.
IMO we should just render the homescreen activity inside searchwindow and at the same time do not hide rocketbar. What's the issue not to show the searchWindow when it's having an activity?

Hey slow down, guys, I am here to help, explain what's your need to me and we will have less trouble back to bite us in the future.

Comment 9

[Kevin Grandon :kgrandon]

Attachment: System Patch

Alive - what about something like this? The UI is still not perfect as it is constrained within the search, but it does function at least and we can follow it up with some UX polish.

Also I need to leave for the night soon, if you're not happy with this I would love for you to implement something that you like better. Thanks!

Comment 10

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

Comment on attachment 8443254 [details]
System Patch

I'd like to make AppWindowManager.init() happens later then rocketbar instantiation in bootstrap, hence we don't need the publish change.

Lemme know if you need me to review again.

Comment 11

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: bug1027990.style.patch

Kevin, here is the necessary bits for the UI to cover the whole screen (on top of bug 1030248).

Comment 12

[Kevin Grandon :kgrandon]

Attachment: Github pull request

Pull request with both of our commits.

Comment 13

[Kevin Grandon :kgrandon]

Comment on attachment 8447622 [details] [diff] [review]
bug1027990.style.patch

R+, but obsoleting in favor of landing the pull request which contains both commits.

Comment 14

[Kevin Grandon :kgrandon]

Comment on attachment 8447673 [details] [review]
Github pull request

I think bug 1030248 solves a few z-indexing issues. Once that lands we should land this as well.

Comment 15

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Kevin Grandon :kgrandon from comment #14)
> Comment on attachment 8447673 [details] [review]
> Github pull request
> 
> I think bug 1030248 solves a few z-indexing issues. Once that lands we
> should land this as well.

Yeah. Landed now.

Comment 16

[Kevin Grandon :kgrandon]

Gaia-try is running now with a rebased PR.

(In reply to Alive Kuo [:alive][NEEDINFO!][OOO until 6/30] from comment #10)
> Comment on attachment 8443254 [details]
> System Patch
> 
> I'd like to make AppWindowManager.init() happens later then rocketbar
> instantiation in bootstrap, hence we don't need the publish change.

I took a look at doing this, but the problem is that rocketbar listeners are async (depending on mozSettings return), so we can not rely on ordering. I think dispatching to the body, or perhaps even app iframe in the future, is reasonable here to not depend on listener attachment order. Let me know if you have any concerns/suggestions.

Comment 18

[Kevin Grandon :kgrandon]

Landed: https://github.com/mozilla-b2g/gaia/commit/90b1e33b84e9e21f8baa8d4fe767eb137a492675

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Bug 1030259 was made 2.0+, but this is the bug that fixes it apparently. Seems like this is the one that should have blocking status.

Comment 20

[Jason Smith [:jsmith]]

Blocks a blocker.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/feb20fa7361f3016888fa872e882a495885a0f94