Closed
Bug 996515
Opened 11 years ago
Closed 10 years ago
[NFC] Screen shot does not have move back animation when video is failed to share
Categories
(Firefox OS Graveyard :: Gaia::System, defect)
Tracking
(feature-b2g:2.0, tracking-b2g:backlog)
VERIFIED
FIXED
2.0 S2 (23may)
People
(Reporter: ashiue, Assigned: gasolin)
References
Details
(Whiteboard: [priority][p=1])
Attachments
(2 files)
Using most up-to-date pvt build (2014/4/14) to test Two phones(Device A, Device B) with NFC STR: 1. Enable NFC in settings 2. Device A open video 3. Touch two phones together 4. Swipe screen shot on device A 5. Remove device A from device B before start to transfer file Expected result: Device A should have screen shot move back animation before screen back to video Actual result: Just back to video without move back animation
Blocks: NFC-Gaia
Updated•11 years ago
|
blocking-b2g: --- → backlog
Updated•11 years ago
|
Whiteboard: [priority]
Comment 2•11 years ago
|
||
Yes, but not now. It may take some time to survey the reason, because I suspect that something broken under the Gaia, so the event didn't send normally. I've discussed this with QA.
Flags: needinfo?(gweng)
Comment 3•11 years ago
|
||
According to Greg, we should already have the UI code ready. We need to verfiy shrinking_ui.js have indeed received the "shrinking-rejected" event in this case. Siddartha, could you check the NFC manager code on this? thanks.
Component: NFC → Gaia::System
Flags: needinfo?(psiddh)
Does this issue get reproduced after multiple attempts? Because, I tried to reproduce the issue with two NFC phones (latest FxOS builds), I was able to reproduce a issue (Not sure if its the same one reported) only after attempting multiple times. The shrinking UI does not move back after 5 -6 attempts on Video app. Upon further analyzing by adding logs, 1) nfc_manager does send 'shrinking-stop' @ http://lxr.mozilla.org/gaia/source/apps/system/js/nfc_manager.js#388 2) Shrinking UI's handle event gets called - http://lxr.mozilla.org/gaia/source/apps/system/js/shrinking_ui.js#125 3) Shrinking UI attempts to stop the tilted UI http://lxr.mozilla.org/gaia/source/apps/system/js/shrinking_ui.js#242 4) However 'this._state()' returns 'false' @ Ln http://lxr.mozilla.org/gaia/source/apps/system/js/shrinking_ui.js#244 and the control returns from the stop() function. As a consequence of this, 'shrinking effect' is not restored (tilted back) and therefore this issue. *Again, I am not sure if this is the same bug that is reported, if not this should be another bug to be raised and fixed. Also please note that there is also a race condition issue around shrinking UI (in Nfc stack). This is being addressed in the bug : https://bugzilla.mozilla.org/show_bug.cgi?id=959059
Comment 7•11 years ago
|
||
In Bug 961687, The send does not happen unless the "confirm" slide animation is complete, but I was only looking at sendNDEF(), not sendFile(blob). However, I think it's very similar, and it may be a relevant. Please retest when it lands.
Updated•11 years ago
|
feature-b2g: --- → 2.0
Comment 8•11 years ago
|
||
(In reply to Garner Lee from comment #7) > In Bug 961687, The send does not happen unless the "confirm" slide animation > is complete, but I was only looking at sendNDEF(), not sendFile(blob). > However, I think it's very similar, and it may be a relevant. Please retest > when it lands. Hi, please retest it. Thanks.
Flags: needinfo?(gweng) → needinfo?(ashiue)
Reporter | ||
Comment 9•11 years ago
|
||
I verified on Gaia 4f352142a54f7f7ae2c460aad9049eda4b0edb14 Gecko https://hg.mozilla.org/mozilla-central/rev/9ac3e2dd0898 BuildID 20140508160203 Version 32.0a1 Please refer this video that still no move back animation when share URL fail.
Flags: needinfo?(ashiue)
Comment 10•11 years ago
|
||
Hi Greg, I think the reproduction steps changed between the bug open, and the switch to URL sharing changed, but it's the same bug. Right now, the animation doesn't chain, because stop does a hard stop on all animations (_cleanEffects). Can you take a look? If it stops, it has to let the tilt animation to be added, which means shrinking-start has to transition from an intermediate "stopping" state, instead of flipping back instantly.
Flags: needinfo?(gweng)
Comment 11•11 years ago
|
||
I remember that I mentioned once...Shrinking UI would do the rejection animation if NFC manager or other components send an event called 'shrinking-rejected', it would perform the animation to revert the session back. This is because the Shrinking UI of course should not judge if the sharing is successful or not. However, I grep the whole System app, no one would send the event. So this may be the first thing we should solved, unless the event would already be fired but it would be handled incorrectly.
Flags: needinfo?(gweng)
Comment 12•11 years ago
|
||
Greg, I've looked into how NfcManager handles NFC events and you're right, it doesn't dispatch the 'shrinking-rejected' event. Me and Krzysiek have discussed it, and we believe it should be called as a result of 'nfc-manager-tech-lost'. Now, 'shrinking-stop' is dispatched instead [1]. However, even if we replace it with 'shrinking-rejected', it wouldn't behave as expected, right? I mean the screenshot would fly back in, but it'll still be tilted, right? So we'd need to either send 'shrinking-stop' shortly after that, or have 'shrinking-rejected' call .stop() once animation finishes. [1] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/nfc_manager.js#L412
Flags: needinfo?(gweng)
Comment 13•11 years ago
|
||
I think it should be modified the handler of 'shrinking-rejected' in shrinking_ui.js, to make it tilt back: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/shrinking_ui.js#L294 This was missed at the first implementation.
Flags: needinfo?(gweng)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → gasolin
Whiteboard: [priority] → [priority][p=1]
Assignee | ||
Comment 14•10 years ago
|
||
* call 'shrinking-rejected' in 'nfc-manager-tech-lost' * have 'shrinking-rejected' call .stop() once animation finishes.
Attachment #8424651 -
Flags: review?(alive)
Attachment #8424651 -
Flags: feedback?(gweng)
Comment 15•10 years ago
|
||
Comment on attachment 8424651 [details]
pull request redirect to github
Test for nfc_manager and shrinking_ui is wanted.
Attachment #8424651 -
Flags: review?(alive) → feedback+
Assignee | ||
Updated•10 years ago
|
Target Milestone: --- → 2.0 S2 (23may)
Comment 16•10 years ago
|
||
Comment on attachment 8424651 [details]
pull request redirect to github
With tests it looks good.
Attachment #8424651 -
Flags: feedback?(gweng) → feedback+
Assignee | ||
Comment 18•10 years ago
|
||
Comment on attachment 8424651 [details]
pull request redirect to github
test added and travis passed, please kindly review it again
Attachment #8424651 -
Flags: review?(alive)
Updated•10 years ago
|
Attachment #8424651 -
Flags: review?(alive) → review+
Assignee | ||
Comment 19•10 years ago
|
||
merged https://github.com/mozilla-b2g/gaia/commit/5a0c617fcb004e1066a2f0326ab86dc204fc11d2 thanks
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 20•10 years ago
|
||
Verified on Gaia c462d9183d294a2d8ecc472f593ea8cfa15bc9de Gecko https://hg.mozilla.org/mozilla-central/rev/9d8d16695f6a BuildID 20140520160203 Version 32.0a1
Status: RESOLVED → VERIFIED
Updated•10 years ago
|
blocking-b2g: backlog → ---
tracking-b2g:
--- → backlog
You need to log in
before you can comment on or make changes to this bug.
Description
•