Closed Bug 877903 Opened 9 years ago Closed 9 years ago
Starting an app in landscape orientation shows screenshot first in portrait
255.07 KB, application/x-zip
187 bytes, text/html
187 bytes, text/html
187 bytes, text/html
When an app launches in landscape,it is is a very noticeable delay between click, startup screenshot and screen rotation. Showing the screenshot in portrait for a longer time looks badly broken. This affects a high-profile partner which reviews the user experience very critically. Might be related to bug 840147.
Reproducible with https://marketplace.firefox.com/app/zombiez Tested Unagi with 126.96.36.199 20130320070207, but also clearly visible on 1.0. Need info from mounir as he was looking into bug 840147.
That's a very old build. Can you reproduce this with the patch included on bug 840147?
This has nothing to do with bug 840147. It sounds like Gaia window manager that should do the locking earlier.
Tested on Unagi - 1.0.1 - 06-Jun-2013 08:40: http://www.youtube.com/watch?v=JaTIhvtTXq0 1st launch bug: Loading screen has ugly white padding 2nd+ launch bug: Snapshot is shown as filling pattern in portrait Task switching bug: Games shrinks while showing portrait mode Tested on Unagi - 1.1 - 06-Jun-2013 08:31: http://www.youtube.com/watch?v=chKgcgBdGMc 1st launch bug: Loading screen has ugly white padding Task switching bug: Games shrinks/grows while showing portrait mode This shows how loading and task switching is badly impacted by visual glitches. As this impacts the approval from a high priority partner I nominate it as blocker.
As mentioned above this affects user experience for landscape oriented games. This is glitch is very visible to a priority partner that has to approve their app as being preloaded.
blocking-b2g: --- → leo?
Component: Gaia → Gaia::System
We would block on this, but at this point we need partner buy-in to mark as blocking, leaving this for partner triage to evaluate.
Assignee: nobody → alive
Exactly bug 840187 solves this problem --- only in master. But because the work depends on some patches only landed in master, it's not uplift-able. What we need here is a v1-train only patch.
(In reply to Alive Kuo [:alive] from comment #7) > Exactly bug 840187 solves this problem --- only in master. Sorry, it should be bug 840147. > But because the work depends on some patches only landed in master, it's not > uplift-able. > What we need here is a v1-train only patch.
Let's write a v1-train patch based on bug 840147... Stealing.
Assignee: alive → rexboy
After testing the test case I found we need some small changes on master. - The top and left position (added for opening animation) should be removed right after opening animation ended. - We should consider apps with orientation just "landscape" rather than landscape-primary and landscape-secondary. I make landscape open in landscape-primary by default.
Patch for v1-train. These codes mostly the same as patch for bug 840147, but the css part changes significantly because the transition differ from master. See comments on Github.
Attachment #762581 - Flags: review?(alive)
Attachment #762581 - Flags: review?(alive) → review+
Comment on attachment 762578 [details] patch (master) r=me if this works when the orientation of app is 'landscape-secondary'. If it doesn't, please use mozOrientation as parameter to decide rotation angle instead of amending '-primary' to non suffix orientation.
Comment on attachment 762578 [details] patch (master) Alive: I've added codes to determine actual device orientation for opening apps. Please take a look, thanks!
Comment on attachment 762578 [details] patch (master) Some notions I remember: 0) Use screen.mozOrientation and mozorientationchange event if possible. 1) Don't use dataset as cross module communication. Use object attributes. app.frame.dataset.orientation -> app.currentOrientation Therefore, please move these into appWindow object.(window.js) (A) dataset.orientation (B) setRotationOrientation (C) setSize ----- if possible and if necessary. See https://github.com/alivedise/gaia/commits/testable-window-manager resize in app_window.js for reference. (Note: do not copy and paste here) 2) Use class as css rules instead of dataset.orientation, e.g., div.appWindow.orientation-landscape. 3) The flicking of orientation lock after window is opened seems strange. Investigate more and file a gecko bug if it's really a gecko bug....(flicking when screen.mozLockOrientation('portrait-primary') to screen.mozLockOrientation(APP_ORIENTATION)) Try remove/add css earlier/later and/or use one time orientationchange event handler. 4) We don't need to calculate deviceorientation every second. Use it on demand when transitioning is happening(appwillopen---->appopen, appwillclose---->appclose) 5) Write some jsdoc comment when you move something into AppWindow and it's public/private attribute/method. private with _ prefix. Sorry for not detailed review last time, but let's refine these in a non-urgent bug and get everything move forward. Thanks.
Comment on attachment 762581 [details] Patch (v1-train) See last comment.
(In reply to Alive Kuo [:alive] from comment #14) > 2) Use class as css rules instead of dataset.orientation, > e.g., div.appWindow.orientation-landscape. Or div.appWindow.rotate-to-landscape-orientation ?
For (3), after some investigating, css for rotating is sometimes removed too early before |orientationchange| raised, so the window looks like flicking. if we don't want to add one additional mozScreenLock() (to make |orientationchange| raise earlier), we may need to listen to |orientationchange| somewhere and remove css there. (I'm not sure where is appropriate for now, because this event doesn't raise necessarily)
Comment on attachment 762578 [details] patch (master) There are significant refactor works so let's review on master first. After that, I will merge changes into v1-train patch.
Comment on attachment 762578 [details] patch (master) Well done! Window Manager's weight decreases! (With some github comments.)
I suggest let's wait some days to see if regression occurs before going to v1-train.
Oops... I'm so sorry. A nit error break the unit test after fixing comments on Github. Alive would you mind reviewing this patch? Thanks a lot!
Comment on attachment 767034 [details] Patch (unit-test regression) r+ if travis goes green after processing.
Attachment #767034 - Flags: review?(alive) → review+
Comment on attachment 767034 [details] Patch (unit-test regression) master https://github.com/mozilla-b2g/gaia/commit/44a7f83aff166b897659531f259e3e1dd3ff27d1 I'll merge v1-train patch tomorrow if no more regression is reported.
Setting to RESOLVED FIXED since merged in master. Just a notice that v1-train patch is different with master one so don't uplift master patch to v1-train.
Oops, I was told the patch cause some problem on Keboard. Backout for now before I fix it. Sorry for this.
Seems it's just some mistyping during resolving conflict. Should be good now. v1-train: https://github.com/mozilla-b2g/gaia/commit/4a5aa95c17656a84395eb78763f7f1c0f9623827 Sorry for the inconvience.
v1.1.0hd: 4a5aa95c17656a84395eb78763f7f1c0f9623827 v1.1.0hd: d55d2d2d2317b09cc61404ec6d23c0346e9e006b v1.1.0hd: 0dcb200ccaa84fc53fea87009911aa2d682131d6
I just backed out this change in Gaia v1-train, be24875471b2a23e61c9deda8e75b13b56e1a116, because it broke the smoke test, Bug 888339. I did not back out the changes in Gaia master, because I could not reproduce Bug 888339 on Gaia master.
Can someone also back this out on 1.1 hd? I'd imagine this bug is reproducing there as well.
Bug 854759 uplifted so let's do it again (the older patch has been backed out.): v1-train https://github.com/mozilla-b2g/gaia/commit/5d7a0da528dce626b563abafc135eb828599d3c0 v1.1.0hd https://github.com/mozilla-b2g/gaia/commit/ad4d7406bf616c468b0472e9ff99e74e08f7b6a2
The patch here works on a 7/7 b2g18 build, but I am seeing a problem with delayed app opening transitions now. See bug 890780 for a followup.
We are also seeing unusual behavior with the contacts picker not being sized correctly when the keyboard is up. See bug 891408.
Depends on: 891408
You need to log in before you can comment on or make changes to this bug.