If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

[System] ActivityWindow does not receive proper rearWindow

RESOLVED FIXED in Firefox OS v2.0

Status

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

People

(Reporter: kgrandon, Assigned: kgrandon)

Tracking

unspecified
2.0 S5 (4july)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.0+, b2g-v2.0 fixed, b2g-v2.1 fixed)

Details

(Whiteboard: [systemsfe])

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

3 years ago
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.
(Assignee)

Comment 1

3 years ago
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!
Flags: needinfo?(aus)
Flags: needinfo?(alive)

Comment 2

3 years ago
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.
Depends on: 1027414
Flags: needinfo?(aus)
(Assignee)

Comment 3

3 years ago
Created attachment 8443228 [details]
Probably a terrible fix

Hey Alive - any better ideas?
Attachment #8443228 - Flags: feedback?(alive)
(Assignee)

Comment 4

3 years ago
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 :)
(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.
Flags: needinfo?(alive)
(Assignee)

Comment 6

3 years ago
Comment on attachment 8443228 [details]
Probably a terrible fix

Uploading a new one
Attachment #8443228 - Flags: feedback?(alive)
(Assignee)

Comment 7

3 years ago
Created attachment 8443236 [details]
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.
Attachment #8443228 - Attachment is obsolete: true
Attachment #8443236 - Flags: feedback?(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.
Attachment #8443236 - Flags: feedback?(alive) → feedback-
(Assignee)

Comment 9

3 years ago
Created attachment 8443254 [details]
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!
Attachment #8443254 - Flags: review?(alive)
(Assignee)

Updated

3 years ago
Attachment #8443236 - Attachment is obsolete: true
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.
Attachment #8443254 - Flags: review?(alive) → review+
Created attachment 8447622 [details] [diff] [review]
bug1027990.style.patch

Kevin, here is the necessary bits for the UI to cover the whole screen (on top of bug 1030248).
Attachment #8447622 - Flags: review?(kgrandon)
(Assignee)

Comment 12

3 years ago
Created attachment 8447673 [details] [review]
Github pull request

Pull request with both of our commits.
Assignee: nobody → kgrandon
Attachment #8443254 - Attachment is obsolete: true
Status: NEW → ASSIGNED
(Assignee)

Comment 13

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

R+, but obsoleting in favor of landing the pull request which contains both commits.
Attachment #8447622 - Attachment is obsolete: true
Attachment #8447622 - Flags: review?(kgrandon)
(Assignee)

Comment 14

3 years ago
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.
Attachment #8447673 - Flags: review+
(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.
OS: Mac OS X → All
Hardware: x86 → All
(Assignee)

Comment 16

3 years ago
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.
(Assignee)

Updated

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

Comment 18

3 years ago
Landed: https://github.com/mozilla-b2g/gaia/commit/90b1e33b84e9e21f8baa8d4fe767eb137a492675
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Assignee)

Updated

3 years ago
Blocks: 1030259
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.
blocking-b2g: --- → 2.0?
status-b2g-v2.0: --- → affected
status-b2g-v2.1: --- → fixed
Flags: needinfo?(bbajaj)
Target Milestone: --- → 2.0 S5 (4july)
Blocks a blocker.
blocking-b2g: 2.0? → 2.0+
Whiteboard: [systemsfe]
v2.0: https://github.com/mozilla-b2g/gaia/commit/feb20fa7361f3016888fa872e882a495885a0f94
status-b2g-v2.0: affected → fixed

Updated

3 years ago
Flags: needinfo?(bbajaj)
Blocks: 1036359

Updated

3 years ago
No longer blocks: 1036359
Depends on: 1036359
You need to log in before you can comment on or make changes to this bug.