1064975 - [Browser Windows Manager] Pressing the Windows Manager button while in Browser does nothing

Status: VERIFIED FIXED

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

Description

[Marcia Knous [:marcia]]

Attachment: taskmanager.txt

Flame, while running:

Gaia   c7b55ed3be126b1f5f3d9cdd0277c365a6652d29
SourceStamp 956b152e6d21
BuildID 20140909000204
Version 34.0a2
base: v123

STR:
1. Open the Browser app with some other windows and tabs open
2. Press the button to the right of the browser URL bar

Expected: task manager would open
Actual: Nothing happens when the button is pressed

Comment 1

[Jason Smith [:jsmith]]

QA Wanted for a video.

Comment 3

[Pi Wei Cheng [:piwei] (inactive)]

I was having a hard time understanding this bug, but the bug you duped to this bug seems much clearer on where the issue is located and how to repro it.

Is this bug talking about the same button as described in bug 1065545 - where when you start a fresh browser session without navigating to any website, the button to the right of URL bar, which looks like two squares stacked together, is unresponsive when tapped on?

Comment 4

[Marcia Knous [:marcia]]

[Blocking Requested - why for this release]: Broken functionality in the Browser App.

In answer to question 3, the bug I duped is using incorrect terminology. Yes, it is the Window Manager button to the right of the URL bar.

https://mozilla.app.box.com/s/jw7wgne4esbg9ccgx00s/1/2243605137/20745078519/1 - Page 18 shows how it supposed to work and has some clear screenshots of the button in question.

Comment 5

[Pi Wei Cheng [:piwei] (inactive)]

(In reply to Jason Smith [:jsmith] from comment #1)
> QA Wanted for a video.

Video added from bug 1065545

http://youtu.be/CoENwd3On9A

Also flipping tracking flags info according to bug 1065545. I also did a check on the first build that Browser2 was introduced on 8/18, and this button was NOT present in that build. I assume this button was added later in Browser2 and was implemented broken therefore it's not a regression.

Comment 6

[Kevin Grandon :kgrandon]

Adding qawanted because I am unable to reproduce this on the latest nightly builds on trunk. Is it still a problem? I'm running: 

Gaia      6cb5e0100d70313e4922c8d34bf20dcdd66ef616
Gecko     https://hg.mozilla.org/mozilla-central/rev/2db5b64f6d49
BuildID   20140912040204
Version   35.0a1
ro.build.date   Fri Jun 27 15:57:58 CST 2014
ro.bootloader   L1TC00011230
ro.build.version.incremental   110

Comment 7

[Pi Wei Cheng [:piwei] (inactive)]

(In reply to Kevin Grandon :kgrandon from comment #6)
> Adding qawanted because I am unable to reproduce this on the latest nightly
> builds on trunk. Is it still a problem? I'm running: 

On latest master I'm also unable to reproduce the issue. When I tap on the Browser Window button without another browser session open, it goes to a window saying 'Your recent browser windows show up here'. It seems odd to me that tapping on that screen will just close out the browser though (I'd expect it to go back to previous screen in browser).

Device: Flame 2.2 Master
BuildID: 20140912061053
Gaia: b72909030e214175144342f7e5df7e88a2b52fd4
Gecko: 59d4326311e0
Version: 35.0a1 (2.2 Master)
Firmware: v165
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

----------

However issue is STILL reproducible on Flame 2.1. Tapping on Browser Window button does nothing.

Device: Flame 2.1
BuildID: 20140912123307
Gaia: d4de4123c678b198692af256254a26507e9c9a08
Gecko: 0807d8ab3e5a
Version: 34.0a2 (2.1)
Firmware: v165
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0

Comment 8

[Kevin Grandon :kgrandon]

Ok, it sounds like we might be missing a patch to uplift, so we just need to investigate what needs to be uplifted here.

Aus - looping you in because you've done some work with the task manager recently. Any idea off the top of your head if we might be missing here in 2.1?

Comment 9

[Aus Lacroix [:aus]]

Sounds to me like there's code missing.

Comment 10

[Kevin Grandon :kgrandon]

Attachment: 2.1 PR - Remove task manager filter

Aus - asking you to review since you implemented the original filter here. It seems that it doesn't work from the browser app in 2.1, but works fine in master.

I couldn't quickly figure it out, so to get it working, I'd suggest just disabling the filter in 2.1. If you'd like to figure out what's going on with the filter feel free to steal/investigate.

Comment 11

[Aus Lacroix [:aus]]

Hmmm, the change to remove this seems good but I will try and fix it first since I have some bandwidth on my end. Yoink!

Comment 12

[Aus Lacroix [:aus]]

Comment on attachment 8489078 [details] [review]
2.1 PR - Remove task manager filter

lgtm but let's only use this as a last resort.

Comment 13

[Aus Lacroix [:aus]]

Attachment: Pull Request - Manually check for evt.detail and filter presence in detail

Comment 14

[Kevin Grandon :kgrandon]

Comment on attachment 8489078 [details] [review]
2.1 PR - Remove task manager filter

Thanks for digging into this.

Comment 15

[Kevin Grandon :kgrandon]

Comment on attachment 8490434 [details] [review]
Pull Request - Manually check for evt.detail and filter presence in detail

This is so weird, but looks good to me. Thanks!

Comment 16

[Aus Lacroix [:aus]]

Comment on attachment 8490434 [details] [review]
Pull Request - Manually check for evt.detail and filter presence in detail

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): New Feature
[User impact] if declined: We'll remove this feature, they will need to find their browser windows amongst all other application windows.
[Testing completed]: On Flame device, latest mozilla-aurora + latest gaia 2.1 + my fix
[Risk to taking this patch] (and alternatives if risky): Zero risk.
[String changes made]: None.

Comment 17

[Aus Lacroix [:aus]]

Commit (v2.1): https://github.com/mozilla-b2g/gaia/commit/3928a7cc9b5330910d1c2154a59f92d9d8010e8c

Fixed.

Comment 18

[Marcia Knous [:marcia]]

I tested this today using a Flame KK build, and the button still doesn't work for me using:

Gaia   3742913e11f69e789dcb0aa0dedf2e5572da0129
SourceStamp df42b05782aa
BuildID 20140923003005
Version 34.0a2
base image: V180 with Full Flash

Bug 1071346#c1 indicates it happens on other configurations as well.

Comment 19

[Kevin Grandon :kgrandon]

Do we need this bug and bug 1071346?

Aus - Can you take a look? Thanks!

Comment 20

[Aus Lacroix [:aus]]

Yep, looking into it already. This works on builds I make myself but apparently not with production builds. :(

Comment 22

[Aus Lacroix [:aus]]

Hah! Since we were filtering out the browser application itself from the stack we were not calling 'enterTaskManager' on it so it was stuck as an active application.

I've modified card.js to *no longer* call enter/leave taskManager and instead have the taskManager do that itself when appropriate *and on all applications always*.

This works great and I'm in the process of adding a test for this.

Comment 23

[Aus Lacroix [:aus]]

Attachment: Pull Request - Ensure enter/leaveTaskManager is called for all applications even when filter is used.

Comment 24

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8493913 [details] [review]
Pull Request - Ensure enter/leaveTaskManager is called for all applications even when filter is used.

Yep, makes sense to me and the same fix is already present and working on master.

Comment 25

[Aus Lacroix [:aus]]

Comment on attachment 8493913 [details] [review]
Pull Request - Ensure enter/leaveTaskManager is called for all applications even when filter is used.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): New feature.
[User impact] if declined: Show Windows button in Main System Browser Window never works.
[Testing completed]: On device, Flame (1G, 319M)
[Risk to taking this patch] (and alternatives if risky): Very low.
[String changes made]: None.

Comment 26

[Aus Lacroix [:aus]]

Fixed in 2.1.

Commit (v2.1): https://github.com/mozilla-b2g/gaia/commit/a4c14a9a385af9957c0958667405d186e0c87a66

Comment 27

[Ryan VanderMeulen [:RyanVM]]

Reverted for Gaia unit test failures. The same ones as your Gaia-Try run no less. Please double-check your results before merging and burning the tree in the future.
v2.1: https://github.com/mozilla-b2g/gaia/commit/4a9bebcd9dcced7625bc6c1362855a8447eb6de0

https://tbpl.mozilla.org/php/getParsedLog.php?id=48797457&tree=Mozilla-Aurora

Comment 28

[Aus Lacroix [:aus]]

Attachment: Pull Request - Ensure enter/leaveTaskManager is called for all applications even when filter is used.

I fixed the bad test (which was fixed locally in a *different* branch of mine :() and also added a test for enter/leave since that stuff moved from the card to being done by the task manager. Approval and review carries. I have a green test run locally. Just waiting on gaia-try.

Comment 29

[Aus Lacroix [:aus]]

Try run: https://tbpl.mozilla.org/?rev=0659c5a4e3215d9593bd608c67bdfbae5eac058a&tree=Gaia-Try

Just waiting on Gij to finish.

Comment 30

[Aus Lacroix [:aus]]

Commit (v2.1): https://github.com/mozilla-b2g/gaia/commit/a7e53d661a1329d6813bc6104555b57dbb5e4d85

Will keep an eye on TBPL until it's green there as well.

Comment 31

[Yeojin Chung [:YeojinC]]

This issue is verified fixed on Flame 2.1:

Flame 2.1 KK (319mb) (Full Flash)

Environmental Variables:
Device: Flame 2.1
BuildID: 20141012001201
Gaia: d18e130216cd3960cd327179364d9f71e42debda
Gecko: 610ee0e6a776
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

The windows manager functions properly.