Closed Bug 1066206 Opened 7 years ago Closed 7 years ago

[Search] When user tried to add a Rocketbar result to the homescreen the page opens silently in the background

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.1+, b2g-v2.0 unaffected, b2g-v2.1 verified, b2g-v2.2 verified)

VERIFIED FIXED
2.1 S5 (26sep)
blocking-b2g 2.1+
Tracking Status
b2g-v2.0 --- unaffected
b2g-v2.1 --- verified
b2g-v2.2 --- verified

People

(Reporter: bzumwalt, Assigned: kgrandon)

References

()

Details

(Keywords: regression, Whiteboard: [2.1-flame-test-run-2][systemsfe])

Attachments

(2 files)

Attached file Logcat
Description:
Tapping Rocketbar on homescreen and typing a string in search bar, then long pressing one of the results brings up the context page. Selecting "add to homescreen" opens the "Add to Homescreen" page in the background. User cannot see this page unless they switch to it by pressing the home button, then pressing Rocketbar again.

Once user switches to page, they can add the bookmark to the homescreen as expected. This process must be repeated for each subsequent bookmark added to homescreen from Rocketbar.
   
Repro Steps:
1) Update a Flame device to BuildID: 20140911000225
2) Tap Rocketbar
3) Type any string
4) Long press a result then select "Add to Homescreen"
  
Actual:
User has to press home button then re-enter rocketbar in order to access "Add to Homescreen" page.
  
Expected: 
"Add to Homescreen" page appears in foreground without further user input.
  
Environmental Variables:
Device: Flame 2.1 (319 mb)
BuildID: 20140911000225
Gaia: d61264cd0c1f797b6be11e33524d8d52983c87e4
Gecko: 1d44dfce2e5b
Version: 34.0a2 (2.1)
Firmware: V123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
  
Repro frequency: 3/3, 100%
Link to failed test case:
See attached: Youtube video clip & logcat
Youtube link: http://youtu.be/CmUGzzUcnzk
Issue DOES occur on Flame 2.2 (319mb), OpenC 2.2, Flame 2.1 (512mb), and OpenC 2.1

Environmental Variables:
Device: Flame Master (319mb)
BuildID: 20140910040203
Gaia: 8e02f689b0fc39cb6ccdc22d02ed7e219c58faa7
Gecko: 152ef25e89ae
Version: 35.0a1 (Master) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Environmental Variables:
Device: Open_C Master
BuildID: 20140910040203
Gaia: 8e02f689b0fc39cb6ccdc22d02ed7e219c58faa7
Gecko: 152ef25e89ae
Version: 35.0a1 (Master) 
Firmware Version: P821A10V1.0.0B06_LOG_DL
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Environmental Variables:
Device: Flame 2.1 (512mb) 
BuildID: 20140910000202
Gaia: 79dc972d637ff5ef7667b231e93118b4ed83ba9c
Gecko: 0890010015a2
Version: 34.0a2 (2.1)
Firmware: V123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Environmental Variables:
Device: Open_C 2.1
BuildID: 20140910000202
Gaia: 79dc972d637ff5ef7667b231e93118b4ed83ba9c
Gecko: 0890010015a2
Version: 34.0a2 (2.1) 
Firmware Version: P821A10V1.0.0B06_LOG_DL
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Actual Results: User has to press home button then re-enter rocketbar in order to access "Add to Homescreen" page.


Issue does NOT occur on Flame 2.0 (319mb) and OpenC 2.0

Environmental Variables:
Device: Flame 2.0 (319mb)
BuildID: 20140910000203
Gaia: 3f4c635106c5364228782d12b1cb76b0c105b971
Gecko: 02a5b9234c13
Version: 32.0 (2.0) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Enviromental Variables:
Device: Open_C 2.0
BuildID: 20140910000203
Gaia: 3f4c635106c5364228782d12b1cb76b0c105b971
Gecko: 02a5b9234c13
Version: 32.0 (2.0)
Firmware: P821A10v1.0.0B06_LOG_DL
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Actual Results: "Add to Homescreen" page appears in foreground without further user input.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(dharris)
Keywords: regression
[Blocking Requested - why for this release]: Clear regression from 2.0 - pressing the "add to home screen" should take you directly to the page which allows you to add it without any other actions.
blocking-b2g: --- → 2.1?
Whiteboard: [2.1-flame-test-run-2] → [2.1-flame-test-run-2][systemfe]
Whiteboard: [2.1-flame-test-run-2][systemfe] → [2.1-flame-test-run-2][systemsfe]
There are a few problems here. On master (0f45989dfa25d15ad3d6e0bef41847e46d0da774), the contextmenu does not even show. Then on commit adcf405, we get the contextmenu, but do not get the dialog appearing. For this bisection, we'll just check on the contextmenu part of things.

[~/workspace/gaia ((no branch, bisect started on adcf405))]$ git bisect good adcf405
[~/workspace/gaia ((no branch, bisect started on adcf405))]$ git bisect bad 0f45989dfa25d15ad3d6e0bef41847e46d0da774
Bisecting: 172 revisions left to test after this (roughly 8 steps)
[77a475a68d3a17ca74cae64bc63e3263b353156a] Merge pull request #23348 from yzen/bug-1030346

git bisect good
Bisecting: 86 revisions left to test after this (roughly 7 steps)
[28266041a9e78abd9bc4ec6a1cc85e9d9097c573] Merge pull request #23851 from RudyLu/keyboard/Bug1062105

git bisect bad
Bisecting: 42 revisions left to test after this (roughly 6 steps)
[14709c76da02d7353f52cce8dafee9d08a6afc18] Merge pull request #23742 from DouglasSherk/1059087-contacts-separator

git bisect bad
Bisecting: 20 revisions left to test after this (roughly 5 steps)
[fe416cc9b2128c3b817134f527233a8bedf99b08] Merge pull request #23841 from empoalp/bug_1062749

git bisect good
Bisecting: 10 revisions left to test after this (roughly 3 steps)
[1bfc67f0e3ef9ef3056f9607392fbb762ae69017] Merge pull request #23731 from eeejay/bug-1062989

git bisect good
Bisecting: 5 revisions left to test after this (roughly 3 steps)
[bc549370546e6d2a8944f12a0e8d2d91b223ef5a] Merge pull request #23848 from daleharvey/1053261

git bisect bad
Bisecting: 1 revision left to test after this (roughly 1 step)
[3f5278adb843f2a8bf93d1890e35f2302cda7b74] Merge pull request #23885 from KevinGrandon/bug_1063694_search_unresponsive

git bisect bad
Bisecting: 1 revision left to test after this (roughly 1 step)
[11e8238cc214e13ec244f15f0978a67f41926198] Bug 1063694 - [System] Fix SearchWindow prototype r=alive

git bisect bad
11e8238cc214e13ec244f15f0978a67f41926198 is the first bad commit
commit 11e8238cc214e13ec244f15f0978a67f41926198
Author: Kevin Grandon <kevingrandon@yahoo.com>
Date:   Tue Sep 9 23:29:32 2014 -0700

    Bug 1063694 - [System] Fix SearchWindow prototype r=alive

Caused by me in bug 1063694.
Blocks: 1063694
Component: Gaia::Search → Gaia::System
Depends on: 1067068
Attached file Github pull request
QA Contact: kgrandon
Target Milestone: --- → 2.1 S5 (26sep)
Depends on: 1066363
No longer depends on: 1067068
Assignee: nobody → kgrandon
Comment on attachment 8489084 [details] [review]
Github pull request

Alive - could you give this a quick review? The problem is that rocketbar is detecting the open-app request and closing rocketbar. By removing/adding the listener we can get around this. Any better ideas?
Attachment #8489084 - Flags: review?(alive)
Comment on attachment 8489084 [details] [review]
Github pull request

My opinion is the same as bug 1065658; it seems we could just drop the open-app listener because the search window will be in the long run closed if the app is asking to open by appopened event. What do you think?
Attachment #8489084 - Flags: review?(alive)
Alive - we can do that, but then we run into bug 1060045 - which is about launching apps. Right now we rely on the system app to manage the search window visibility for us when we receive launch events. We could probably do that in rocketbar.js instead and manually close ourself, but I think that we would probably see some flashing of the backgrounded app which might not be too good.

I think I looked at using appopened before, but this did not solve 1060045. Alive - can you think of some way to solve this and bug 1060045 while using the appopened event?
Flags: needinfo?(alive)
(In reply to Kevin Grandon :kgrandon from comment #8)
> Alive - we can do that, but then we run into bug 1060045 - which is about
> launching apps. Right now we rely on the system app to manage the search

> 
> I think I looked at using appopened before, but this did not solve 1060045.
> Alive - can you think of some way to solve this and bug 1060045 while using
> the appopened event?

Sorry I didn't notice and think of that, hmm...
I think we made some mistake in that bug without a deep consideration, now let's try to resolve it in another way.

The reason of appopened is not published for an already opened app is because this will trigger too many handlers to do redundant work.

The reason of not using open-app is already described above.

> We could probably do
> that in rocketbar.js instead and manually close ourself, but I think that we
> would probably see some flashing of the backgrounded app which might not be
> too good.

Honestly we don't do anything (wait the active app window to repaint) right now when rocketbar is shown/hidden.  We will do in bug 1055299 but still not waiting. Flashing is expected and it's a gecko visibility API design issue.

So the only way to resolve this bug and bug 1055299 at the same time is
* do not use open-app listener.
* do window.close() in search app once it fires the activity to invoke marketplace.
* do not do window.close() in search app if it's adding bookmark.

Please note you need to take care if AppWindow.prototype._handle_mozbrowserclose work for searchWindow. If not we may need to override.

To prevent flashing it's another big issue later in the hierarchy management so let's not address it in this bug.
Flags: needinfo?(alive)
(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #9)
> So the only way to resolve this bug and bug 1055299 at the same time is

I mean bug 1060045 ;)
Comment on attachment 8489084 [details] [review]
Github pull request

I think your suggestion about not processing open-app is the safest thing to do here, and least amount of code to introduce. 

Also added a unit + marionette test to catch this in the future. Could you take a look again? Thanks!
Attachment #8489084 - Flags: review?(alive)
Comment on attachment 8489084 [details] [review]
Github pull request

r=me
Attachment #8489084 - Flags: review?(alive) → review+
Regression
blocking-b2g: 2.1? → 2.1+
Master: https://github.com/mozilla-b2g/gaia/commit/ea06ae56236a06322d8dfe8dd1488b09293dd571
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Comment on attachment 8489084 [details] [review]
Github pull request

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Feature work.
[User impact] if declined: Users will be unable to use this feature.
[Testing completed]: Manual and unit testing.
[Risk to taking this patch] (and alternatives if risky): Low risk. Smallish patch and it's already broken.
[String changes made]: None.
Attachment #8489084 - Flags: approval-gaia-v2.1?(fabrice)
Attachment #8489084 - Flags: approval-gaia-v2.1?(fabrice) → approval-gaia-v2.1+
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(dharris)
I've finally landed the test that was originally part of the PR (but split out due to intermittent issues). It turns out that the intermittent issues were solved in bug 1071349.

https://github.com/mozilla-b2g/gaia/commit/861993dfe6af49435ecb8cf6d1ec2fde0b10ccce
This issue is verified fixed on Flame 2.1 and Flame 2.2.

The "Add to Homescreen" page will appear correctly when the user long taps on a search result.

Flame 2.2 

Device: Flame 2.2 Master KK (319mb) (Full Flash)
BuildID: 20141017040208
Gaia: abef62c0623e5504a97b4fd411e879a67b285b52
Gecko: ae1dfa192faf
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 36.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Flame 2.1 

Device: Flame 2.1 KK (319mb) (Full Flash)
BuildID: 20141017001201
Gaia: 1ea74943cfe525c76a074ca1d7de8e51a70f6b98
Gecko: 2befa902ff5c
Gonk: 05aa7b98d3f891b334031dc710d48d0d6b82ec1d
Version: 34.0 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Flags: in-testsuite?
Flags: in-testsuite+
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Depends on: 1088352
You need to log in before you can comment on or make changes to this bug.