Closed Bug 977934 Opened 6 years ago Closed 6 years ago

[Sora][Message][MMS]Can't enter"Setting"menu when view a picture in MMS.

Categories

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

defect

Tracking

(blocking-b2g:1.3+, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed)

RESOLVED FIXED
1.4 S3 (14mar)
blocking-b2g 1.3+
Tracking Status
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed

People

(Reporter: sync-1, Assigned: alive)

References

()

Details

(Keywords: regression)

Attachments

(5 files, 3 obsolete files)

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:
blocking-b2g: --- → 1.3?
Can someone on Moz side confirm this on the latest 1.3 device?
Keywords: qawanted, regression
Tim, can you take a look here?
Flags: needinfo?(timdream)
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
Keywords: qawanted
QA Contact: mvaughan
Dear sirs,
 
 Have any news about this PR?
Alive,

Please provide review here
blocking-b2g: 1.3? → 1.3+
Flags: needinfo?(alive)
Assignee: nobody → alive
Flags: needinfo?(alive)
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
Component: Gaia::System → Gaia::System::Window Mgmt
Flags: needinfo?(timdream)
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.
We are having the same issue for master.
To avoid regression, I propose to kill the activity once its callee is closed by other app's open.
(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..
Attached file Patch for v1.3
Attachment #8385897 - Flags: review?(etienne)
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!
Attachment #8385893 - Flags: review?(etienne)
Comment on attachment 8385897 [details] [review]
Patch for v1.3

I'll re-review the updated version :)
Attachment #8385897 - Flags: review?(etienne)
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.
Attachment #8385893 - Flags: review?(etienne)
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.
Attachment #8385897 - Flags: review?(etienne)
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).
Attachment #8385893 - Flags: review?(etienne)
Flags: needinfo?(tchung)
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.
Flags: needinfo?(tchung)
Keywords: qawanted
Attached file master v3
Hopefully last round!
Attachment #8385893 - Attachment is obsolete: true
Attachment #8387359 - Flags: review?(etienne)
Comment on attachment 8387359 [details] [review]
master v3

all good, let's just wait for the QA feedback before landing it (Thanks Jason!).
Attachment #8387359 - Flags: review?(etienne) → review+
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 :)
Attachment #8385897 - Flags: review?(etienne) → review+
(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
Keywords: qawanted
I will wait Jason's input here for activity testing until Monday.
Flags: needinfo?(jsmith)
(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.
Flags: needinfo?(jsmith)
master
https://github.com/mozilla-b2g/gaia/commit/d4760bae7d2317d588102c3c84779054b76c3a32

Let's wait for one day before ask v1.3 approval.
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: needinfo?(alive)
Resolution: --- → FIXED
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.
Flags: needinfo?(alive)
(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
Attached file fix-977934 (obsolete) —
Patch for gecko: change the way know the first app is loaded.
Attachment #8388375 - Flags: feedback?(vyang)
Attached file Patch for mozilla-central v2 (obsolete) —
Attachment #8388375 - Attachment is obsolete: true
Attachment #8388375 - Flags: feedback?(vyang)
Attachment #8388376 - Flags: feedback?(vyang)
Attachment #8388376 - Attachment is obsolete: true
Attachment #8388376 - Flags: feedback?(vyang)
Attachment #8388380 - Flags: feedback?(vyang)
Comment on attachment 8388380 [details]
Patch for mozilla-central v3

Let's use gaia fix here.
Attachment #8388380 - Flags: feedback?(vyang)
revert: 2bb8e31a7236ab114c198cd577f404721c36d58d
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached file Patch for master v4
Sadly the webapi testing is using mozbrowserloadend to identify the first app is loaded so we cannot block the event for app itself.
Attachment #8388407 - Flags: review?(etienne)
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)
Attachment #8388407 - Flags: review?(etienne) → review+
https://github.com/mozilla-b2g/gaia/commit/41d4cb48f5ff7f66ab5b6c1ac16203be06a62fb1
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Merge mozilla patch.
Please request approval-gaia-v1.3 when this is ready for uplift to 1.3.
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.
Attachment #8385897 - Flags: approval-gaia-v1.3?
Attachment #8385897 - Flags: approval-gaia-v1.3? → approval-gaia-v1.3+
v1.3: a8e31a9067e999322e7e23959795b9ba44a5a4e5
Target Milestone: --- → 1.4 S3 (14mar)
Duplicate of this bug: 941975
Flags: in-moztrap?
Flags: in-moztrap? → in-moztrap+
Depends on: 1013166
No longer depends on: 1013166
You need to log in before you can comment on or make changes to this bug.