sync-1 Reporter
	Description • 10 years ago

Firefox OS v1.3
  Mozilla build ID: 20140208004002
 
 DEFECT DESCRIPTION:
 ->MS can't enter the "Settings"menu.
 
  REPRODUCING PROCEDURES:
 ->Enter"Messages"to select a MMS which include picture to view;
 ->Tap the picture to view,pull down status bar,tap the"Settings"icon to enter;
 ->MS can't enter the "Settings"menu.(ko)
 
 Regression:Beetle lite FF isn't exist this behavior.
 
  EXPECTED BEHAVIOUR:
 ->Should enter "Settings"menu normally.
 
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:
 
  REPRODUCING RATE:100%
 
  For FT PR, Please list reference mobile's behavior:

	Jason Smith [:jsmith]
	Comment 1 • 10 years ago

Can someone on Moz side confirm this on the latest 1.3 device?

	Gregor Wagner [:gwagner]
	Comment 2 • 10 years ago

Tim, can you take a look here?

	Matthew Vaughan
	Comment 3 • 10 years ago

This issue does reproduce for me on the 02/27/14 1.3 build on a Buri. I can only get this to reproduce when the Settings menu is open before running through the STR.

Device: Buri v1.3 MOZ RIL
BuildID: 20140228004002
Gaia: 9e439c98a05bde90b571701ef0b2dbb4249ef196
Gecko: 7fb42ba60248
Version: 28.0
Firmware Version: V1.2-device.cfg

I was able to get the described issue using these STR:

1) Update Buri to 02/27/14 1.3 BuildID: 20140228004002
2) Launch the Message app
3) Send or receive a MMS
4) Tap on the image
5) Pull down the notification bar
6) Tap the 'Settings' button (gear icon)
7) Once the Settings menu loads, navigate back to the Message app
8) Pull down the notification bar
9) Tap the 'Settings' button (gear icon)

Actual:
The first time this is done, the Settings menu displays for a moment and then disappears. Any time after that, so long as the Settings menu is not closed, the Settings menu will not display.

Attached: SettingMenu.mpeg

	sync-1 Reporter
	Comment 4 • 10 years ago

Dear sirs,
 
 Have any news about this PR?

	Preeti Raghunath(:Preeti)
	Comment 5 • 10 years ago

Alive,

Please provide review here

	Matthew Vaughan
	Comment 6 • 10 years ago

This issue started reproducing on the 11/01/13 1.3 build.

- Last Working -
Device: Buri v1.3 MOZ RIL
BuildID: 20131031040201
Gaia: 412fd463bcb81f0e8bebf6d32500d0c02712748d
Gecko: f0d363d72753
Version: 28.0a1
Firmware Version: V1.2-device.cfg

- First Broken -
Device: Buri v1.3 MOZ RIL
BuildID: 20131101040203
Gaia: ccdf357ea150fc7d8b8a4b74c7adf31e7a57e465
Gecko: abe6790a5dd8
Version: 28.0a1
Firmware Version: V1.2-device.cfg


*This looks to be a gaia issue*

last working gaia/first broken gecko = NO REPRO
Gaia: 412fd463bcb81f0e8bebf6d32500d0c02712748d
Gecko: abe6790a5dd8

first broken gaia/last working gecko = Blocked (image does not appear in the MMS after sending/receiving it)
Gaia: ccdf357ea150fc7d8b8a4b74c7adf31e7a57e465
Gecko: f0d363d72753

Push Log: https://github.com/mozilla-b2g/gaia/compare/412fd463bcb81f0e8bebf6d32500d0c02712748d...ccdf357ea150fc7d8b8a4b74c7adf31e7a57e465

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 7 • 10 years ago

Root cause is z-index issue.
Settings app has the same z-index as gallery activity but the activity is new in the DOM tree so settings won't display.

For v1.5 there's a WIP patch to render activity window inside its caller window so this won't happen,
but for v1.3 I need to find a safer way to resolve this bug.

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 8 • 10 years ago

We are having the same issue for master.

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 9 • 10 years ago

To avoid regression, I propose to kill the activity once its callee is closed by other app's open.

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 10 • 10 years ago

(In reply to Alive Kuo [:alive][NEEDINFO!][:艾莉芙] from comment #9)
> To avoid regression, I propose to kill the activity once its callee is
> closed by other app's open.

Hmm.. This doesn't resolve the problem. Maybe the only way is just render activity inside the caller..

	Etienne Segonzac (:etienne)
	Comment 13 • 10 years ago

Comment on attachment 8385893 [details]
Patch for master: render activity inside caller

Thought about it a bit, and indeed rendering the activity inside the caller looks like our safest bet here.

Made some comments on github, and we need a small test asserting the containerElement for inline activities.

Good news is, the new window manager is really paying off here!

	Etienne Segonzac (:etienne)
	Comment 14 • 10 years ago

Comment on attachment 8385897 [details] [review]
Patch for v1.3

I'll re-review the updated version :)

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 15 • 10 years ago

Comment on attachment 8385893 [details]
Patch for master: render activity inside caller

Spend some time to figure out we should stop events of inner window to be propagated to outer window.

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 16 • 10 years ago

Comment on attachment 8385897 [details] [review]
Patch for v1.3

Note: I didn't clean the rules in system.css in v1.3 but just made it work.

	Etienne Segonzac (:etienne)
	Comment 17 • 10 years ago

Comment on attachment 8385893 [details]
Patch for master: render activity inside caller

I think the event propagation part is to critical to do this way. (ie. having the |if ('stopPropagation' in evt)| to play nice with the tests)

Instead of plain objects to fake events in app_window_test.js we should have a simple mock (even inlined in the file) with the the stopPropagation function defined, _and_ have a test where we spy on it to make sure it's called.

In the meantime since the follow up will only be test-related it would be really helpful to get a QA spin on this patch (strictly activity-focused).

	Jason Smith [:jsmith]
	Comment 18 • 10 years ago

Note - Tony is on PTO for this week & next week. If you have any QA requests, then feel free to ping me directly.

I'll put qawanted to do testing on the pull request seen here.

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 19 • 10 years ago

Hopefully last round!

	Etienne Segonzac (:etienne)
	Comment 20 • 10 years ago

Comment on attachment 8387359 [details] [review]
master v3

all good, let's just wait for the QA feedback before landing it (Thanks Jason!).

	Etienne Segonzac (:etienne)
	Comment 21 • 10 years ago

Comment on attachment 8385897 [details] [review]
Patch for v1.3

and let's wait for the patch to land on the master branch before landing this one :)

	Matthew Vaughan
	Comment 22 • 10 years ago

(In reply to Jason Smith [:jsmith] from comment #18)
> Note - Tony is on PTO for this week & next week. If you have any QA
> requests, then feel free to ping me directly.
> 
> I'll put qawanted to do testing on the pull request seen here.

This issue no longer reproduces for me on the 03/07/14 1.4 build on a Buri when applying Alive's master v3 patch.

Device: Buri v1.4 MOZ RIL
BuildID: 20140307040203
Gaia: 2dba34cf61e8279d0c2dbdf45faa029e3ed24c9e
Gecko: b0e7f63c2138
Version: 30.0a1
Firmware Version: V1.2-device.cfg

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 23 • 10 years ago

I will wait Jason's input here for activity testing until Monday.

	Jason Smith [:jsmith]
	Comment 24 • 10 years ago

(In reply to Alive Kuo [:alive][NEEDINFO!][:艾莉芙] from comment #23)
> I will wait Jason's input here for activity testing until Monday.

You should be good to land - comment 22 indicates that the 1.4 patch was working as expected.

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 25 • 10 years ago

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

Let's wait for one day before ask v1.3 approval.

	Vicamo Yang [:vicamo][:vyang]
	Comment 26 • 10 years ago

https://tbpl.mozilla.org/?tree=B2g-Inbound&rev=7f707aab2138 blow out all Marionette-webapi test cases.  It's previous merge https://github.com/mozilla-b2g/gaia/commit/8c9191d works just fine.

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 27 • 10 years ago

The root cause is we'are stopping mozbrowser event to be propagated to top window in this patch but sadly there is a test in gecko side is using that event.

Proposed fix:
(1) Propagate mozbrowser event to window if we're root window.
(2) Fix gecko to use apploadend / homescreenloadend event instead.

	Carsten Book [:Tomcat]
	Comment 28 • 10 years ago

(In reply to Alive Kuo [:alive][NEEDINFO!][:艾莉芙] from comment #27)
> The root cause is we'are stopping mozbrowser event to be propagated to top
> window in this patch but sadly there is a test in gecko side is using that
> event.
> 
> Proposed fix:
> (1) Propagate mozbrowser event to window if we're root window.
> (2) Fix gecko to use apploadend / homescreenloadend event instead.

this is closing currently b2g-inbound, not sure how long a fix would take maybe fastest way would to backout/revert this fix to be able to reopen the tree

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 29 • 10 years ago

Patch for gecko: change the way know the first app is loaded.

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 32 • 10 years ago

Comment on attachment 8388380 [details]
Patch for mozilla-central v3

Let's use gaia fix here.

	Arthur Chen [:arthurcc][inactive after 6/30]
	Comment 33 • 10 years ago

revert: 2bb8e31a7236ab114c198cd577f404721c36d58d

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 34 • 10 years ago

Sadly the webapi testing is using mozbrowserloadend to identify the first app is loaded so we cannot block the event for app itself.

	Etienne Segonzac (:etienne)
	Comment 35 • 10 years ago

Comment on attachment 8388407 [details] [review]
Patch for master v4

all good.
it will event be pretty clean once we have bug 916709.

(need to update the homescreenWindow test though to make travis happy)

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 36 • 10 years ago

https://github.com/mozilla-b2g/gaia/commit/41d4cb48f5ff7f66ab5b6c1ac16203be06a62fb1

	sync-1 Reporter
	Comment 37 • 10 years ago

Merge mozilla patch.

	Ryan VanderMeulen [:RyanVM]
	Comment 38 • 10 years ago

Please request approval-gaia-v1.3 when this is ready for uplift to 1.3.

	(inactive after 6/18) Alive Kuo [:alive] Assignee
	Comment 39 • 10 years ago

Comment on attachment 8385897 [details] [review]
Patch for v1.3

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined:
[Testing completed]:
[Risk to taking this patch] (and alternatives if risky):
[String changes made]:

Patch on master does not cause regression after landed for one day, request to land v1.3 patch.

	Ryan VanderMeulen [:RyanVM]
	Comment 40 • 10 years ago

v1.3: a8e31a9067e999322e7e23959795b9ba44a5a4e5