Gaia 90605754e9bdbe20f3999522f9e1aec600c422a4 Gecko https://hg.mozilla.org/releases/mozilla-aurora/rev/0bffc7e5d8a2 BuildID 20140702160207 Version 32.0a2 STR: 1. Phone A receive an image via bluetooth 2. Open the image via notification 3. Tap 2 phones together 4. Check the screen Expected result: Shrinking UI show correctly and the image can be shared Actual result: No shrinking UI so that the image cannot be shared

Discussed with Greg, he will help to check why shrinking-ui isn't working here.

Leave NI until I can handle this bug. I'm now in LockScreen mode, and need some time to do the context switching.

I've tried this twice and get different results: the first time I tried it, it got failed as you described. However, the second time I tried it, it success as it should be. And I now suspected that it may be a Blutooth issue because I even encountered Blutooth failure without NFC, and you need to retry it to make the transferring works.

Hi, this bug is recorded for that we cannot share image which is opened via notification. About the BT transfer issue, I will try to reproduce it, thanks.

(In reply to Greg Weng [:snowmantw][:gweng][:λ] from comment #3) > I've tried this twice and get different results: the first time I tried it, > it got failed as you described. Did you find anything wrong, or any information can help us to find the root cause or owner for this ?

Assign to Greg now, let's discussion offline if reassign needed, thanks.

The problem is when you click on the image it transferred, the image you opened is not real Gallery app, but a activity window, which would let Shrinking UI still think the current app is the app launched before your clicking. So, it can't be shared.

Some issues for this design: 1. The activity window screen would mislead user to think this is shareable since they can share image/video in apps. 2. If the current launched app before user click the notification is a contact (another image or browser) which can be shared, and then tap two phones together would show shrink UI (but the screenshot is the activity window screen); after swipe the screen to share, the receiver would feel confused that what he received was not what he saw on sender's screen. ni? Juwei to look into further.

I guess in general users won't differentiate activity window or gallery window, they are simply want to share the image right in front of the screen. So I have 3 proposals: 1. Share the screen: My first though is to make the image shareable, so either we make the activity window shareable or we make the notification access the detail view of gallery app. but I'm not quite sure if it's possible to share the activity window...I need more advices from developers. And, is there any reason that notification only calls out activity window rather than a true app? Is there any constraint that we cannot open an app through notification? 2. Do not share the screen, but we pops out an error toast to inform users that the screen is not shareable: I'll consider it as an alternate solution, and I'm wonder if we can implement it on schedule due to L10 translations. 3. Do not share the screen, but visually the activity window should look very different from gallery detail view: It needs visual design inputs for this solution, but I guess the effort will be in gallery app side. I think the first solution might be the best one in terms of a long stage experience. The 2nd & 3rd solutions are not fully satisfy the experience we expect....but we still can discuss the feasibility base on the schedule now. Let me know your thought, thanks !

NI Alive because we've a offline discussion about activity.

A proposed fix would be: [System] if we see the activity caller is system app, let's invoke it as window disposition activity. otherwise we use the activity disposition in its manifest. [Gallery] Put the system message handler of view in gallery main page also. Possible issues: This change in system will affect all inline activities. Some inline activity may don't want be launched as an app. The system message handler may only lives in its inline page (for example, view.html). Sometimes system is playing the role of launching activity by the current active app. For example, if we click an image anywhere to show the option menu (this feature is not opened yet), system app will call share activity for the app who has the image, but the caller indeed is the app itself. https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/browser_context_menu.js#L195 In the open-app event handler of system app in this case we will see the caller is system app but actually it is not. I dare not to say this is do-able but we need more investigation to the impact it brings us.

Hi, I'm setting target milestone to sprint6 in order to meet 0 NFC bugs before FC. Let me know if you feel uncomfortable at this timeframe.

(In reply to Wesley Huang [:wesley_huang] from comment #12) > Hi, I'm setting target milestone to sprint6 in order to meet 0 NFC bugs > before FC. > Let me know if you feel uncomfortable at this timeframe. Hey, this is a feature request. Due to the risk I don't think it's blocking release. Could we renom?

In my opinion, current situation is the second solution that Juwei mentioned. The only difference is that we indicate user the sharing function doesn't work through not shrinking UI. So I want to suggest keeping what we have now, and do long term improvement in future release.

What's the UX spec about the suggestion 2? A simple notification when user close it with other devices together?

I think it is a little late to have a new implementation. I wonder if UX could accept comment 14.

I just sat down with Juwei and Greg to play with current 2.0 on Flame. This actually is a new feature/scenario which we didn't consider. Based on 2.0 schedule, it's not practical to go for neither localization nor visualization at this moment. Let's defer it to 2.1 and target on the proposal1 from comment 9. (I'm nominating 2.1? to triage). There will be efforts on system app, contact app, and also gallery app.

According to comment 17, remove ni?.

This bug is no longer in 2.1.

Adam, you need to update the tracking flag.

target 2.1

Hit send too fast. NI for Bruce for inline activity discussion planning. Bruce, please check when this can be targeted?

I recall that UX made decision that it should launch contact app instead of inline activity. ni? Juwei to reconfirm this.

As an agreement with Wesley & Greg in comment 9, we are now looking for the possibility of solution 1: launch contact app directly from notification.

feature not blocking, remove blocking flag.

If the issue in this bug is just that we need the gallery view activity to handle NFC, in the same way that the gallery handles it, I suspect that is easy to do. Unless there is an issue where we can't initiate an NFC transfer from inside an inline activity... Punam: since you've worked on other NFC-related gallery bugs, could you see if you can make Gallery's view activity handler respond to add an onpeerready callback and make it work with NFC? Or maybe even better... I wonder if we could add that directly to MediaFrame. Then it would work in the camera app's preview mode, too. (I'm asking you to do this with the expectation that it is only a half day's work... If it is more complicated than that, please just report your findings here and we can discuss.)

Hi David Please find below PR with onpeerready callback added to open inline activity. https://github.com/mozilla-b2g/gaia/pull/22296 Here are my findings (after testing the patch and synch up with alive on IRC) 1. The attached patch fixes NFC sharing for open inline activity, if the gallery app is opened before clicking notification to view image. 2. System app only sends the event to the foreground 'app'. If gallery app is not the foreground app, open inline activity to view image doesn't trigger NFC share as it's not receiving peerready event. 3. If foreground app is browser (or contacts app) and user clicks on notification to view image, on tapping two phones it shrinks the view image UI, but shares the browser URL (#comment 8). This behavior is replicable in both master and with the patch and misleading for the user. In addition to the above PR, we need system app to be able to handle inline activity when pairing NFC. Setting NI flag for alive to share the findings and for his inputs on system app support to fix this issue.Thanks

Hi Punam, I quickly made a patch to support NFC pairing for inline activity, https://github.com/mozilla-b2g/gaia/pull/22334 Lemme know if this works with your patch. Thanks.

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #28) > Hi Punam, I quickly made a patch to support NFC pairing for inline activity, > https://github.com/mozilla-b2g/gaia/pull/22334 > > Lemme know if this works with your patch. Thanks. Hi Alive, I tested 22334 PR with my patch on flame with both latest master build and buildid:20140720160202 and it didn't help. With 22334, NFC sharing was not working for gallery, video or browser app. Thanks

(In reply to Punam Dahiya from comment #29) > (In reply to Alive Kuo [:alive][NEEDINFO!] from comment #28) > > Hi Punam, I quickly made a patch to support NFC pairing for inline activity, > > https://github.com/mozilla-b2g/gaia/pull/22334 > > > > Lemme know if this works with your patch. Thanks. > > Hi Alive, I tested 22334 PR with my patch on flame with both latest master > build and buildid:20140720160202 and it didn't help. With 22334, NFC sharing > was not working for gallery, video or browser app. > Thanks OK, will investigate what happens.. sorry for that.

Could you steal and finish the patch here? Thanks.

we should let this.current = AppWindowManager.getActiveApp() , instead of listening to appcreated and get part of information from it.

Hi Alive, could I have you feedback on this patch before updating tests? I've changed below items 1. shrinkingUI.current will refer to AppWindowManager.activeApp() 2. wrapper will refer to activeApp's bottomMost window 3. shrinkingUI will listen activitiyopen & activityterminate to updateActiveApp 4. add onpeerready event in gallery's open.js

Comment on attachment 8473374 [details] PR to master Works for me, please feedback=greg as well. And I think we need gallery folks to review the gallery change.

Comment on attachment 8473374 [details] PR to master Hi Greg , could you have your feedback on this patch? (comment 33) Thanks.

Comment on attachment 8473374 [details] PR to master Hi Punam, I've added nfc onpeer method in open.js. could I have your feedback on this patch? Thanks.

Comment on attachment 8473374 [details] PR to master Thanks for the patch. I have left comments in the PR on when to set NFC Sharing to false. With those updates, PR has my feedback+ for gallery changes.

Comment on attachment 8473374 [details] PR to master Here comes the 3rd f+. Who should review that?

Comment on attachment 8473374 [details] PR to master Hi Alive, I've added tests and addressed issues. Please kindly review this patch, thanks.

Comment on attachment 8473374 [details] PR to master Hi Yoshi, could I have your feedback in gallery/js/open.js ? Thanks.

Comment on attachment 8473374 [details] PR to master I would encourage you to use element everywhere instead of frame to be consistent but up to you.

Thanks, test looks fine, master: https://github.com/mozilla-b2g/gaia/commit/0e9ccaa0d9edf6c212c9618c656c14f3d3455bc5

Verified on Gaia 6e804a42ab90f4251c7fe8c68731dc1c6abd8006 Gecko https://hg.mozilla.org/mozilla-central/rev/0753f7b93ab7 BuildID 20140826181551 Version 34.0a1