Closed Bug 877903 Opened 11 years ago Closed 11 years ago

Starting an app in landscape orientation shows screenshot first in portrait

Categories

(Firefox OS Graveyard :: Gaia::System, defect)

ARM
Gonk (Firefox OS)
defect
Not set
critical

Tracking

(blocking-b2g:leo+, b2g18 verified, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix, b2g-v1.1hd fixed)

RESOLVED FIXED
1.1 QE3 (26jun)
blocking-b2g leo+
Tracking Status
b2g18 --- verified
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- wontfix
b2g-v1.1hd --- fixed

People

(Reporter: Harald, Assigned: rexboy)

References

Details

(Whiteboard: [mozilla-triage] [LeoVB-])

Attachments

(4 files, 1 obsolete file)

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 1.1.0.0 20130320070207, but also clearly visible on 1.0. Need info from mounir as he was looking into bug 840147.
Flags: needinfo?(mounir)
That's a very old build. Can you reproduce this with the patch included on bug 840147?
Flags: needinfo?(hkirschner)
This has nothing to do with bug 840147. It sounds like Gaia window manager that should do the locking earlier.
Flags: needinfo?(mounir)
Flags: needinfo?(hkirschner)
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.
Flags: needinfo?(alive)
Whiteboard: [mozilla-triage]
blocking-b2g: leo? → leo+
Assignee: nobody → alive
Flags: needinfo?(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
Attached file patch (master) (deleted) —
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.
Attachment #762578 - Flags: review?(alive)
Attached file Patch (v1-train) (deleted) —
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.
Attachment #762578 - Flags: review?(alive) → review+
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!
Attachment #762578 - Flags: feedback?(alive)
Attachment #762581 - Flags: feedback?(alive)
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.
Attachment #762578 - Flags: review+
Attachment #762578 - Flags: feedback?(alive)
Comment on attachment 762581 [details] Patch (v1-train) See last comment.
Attachment #762581 - Flags: review+
Attachment #762581 - Flags: feedback?(alive)
(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)
Target Milestone: --- → 1.1 QE3 (24jun)
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.
Attachment #762578 - Flags: review?(alive)
Comment on attachment 762578 [details] patch (master) Well done! Window Manager's weight decreases! (With some github comments.)
Attachment #762578 - Flags: review?(alive) → review+
I suggest let's wait some days to see if regression occurs before going to v1-train.
Comment on attachment 762581 [details] Patch (v1-train) Updated patch for v1-train. There were a little bit more works on resolving conflicts. As last patch, Javascript part are mostly the same with master, CSS part varies significantly but mostly the same as last one. So I think no bother setting another review, I can set it if someone think it's needed though.
Attached file Patch (unit-test regression) (obsolete) (deleted) —
Attached file Patch (unit-test regression) (deleted) —
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!
Attachment #767031 - Attachment is obsolete: true
Attachment #767034 - Flags: review?(alive)
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.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
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.
Keywords: verifyme
QA Contact: jsmith
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.
Keywords: verifyme
Can someone also back this out on 1.1 hd? I'd imagine this bug is reproducing there as well.
Depends on: 854759
Flags: needinfo?(ryanvm)
Flags: needinfo?(ryanvm) → needinfo?(jhford)
Keywords: verifyme
Depends on: 890780
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
Whiteboard: [mozilla-triage] → [mozilla-triage] [LeoVB+]
v1.1.0hd: 5d7a0da528dce626b563abafc135eb828599d3c0
Whiteboard: [mozilla-triage] [LeoVB+] → [mozilla-triage] [LeoVB-]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: