If you select a picture and then change orientations to landscape you see the parts of the previous and next pictures in gallery. Steps to replicate 1. Click on a picture in thumbnails list 2. Change orientation to landscape Actual: Parts of previous and next picture shows Expected: Picture should open centered in landscape mode without showing parts of previous and next picture This issue is replicable in below environment Os Version - 126.96.36.199 - prerelease BuildId:20140117041037 Platform - 29.0a1 Device - Hamachi Gaia Master - 811110c87e1c985c564210d6ca3797f42f0dd0e2 This issue is not seen in v1.3
On investigating this issue seems to be fallout of flatfish changes in bug 928254 As part of flatfish changes, we are using screenlayoutchange instead of resize handler to handle change in orientation. We are using setFramesPosition to position next and previous frame correct distance from current frame https://github.com/mozilla-b2g/gaia/commit/8521e1f89e55b5b2508a0bcec878e3505b1e800b#diff-508f3a8a4cb447602009081be5eb9236R1208 The issue is when setFramesPosition method is called from screenlayoutchange handler, it's not able to calculate the correct window.innerWidth for that orientation https://github.com/mozilla-b2g/gaia/blob/master/apps/gallery/js/frames.js#L39 I propose using screenlayoutchange handler for changes specific to video https://github.com/mozilla-b2g/gaia/commit/8521e1f89e55b5b2508a0bcec878e3505b1e800b#diff-508f3a8a4cb447602009081be5eb9236R1195 and keep resize handler for handling resizeFrames and setFramesPosition.
Created attachment 8363796 [details] [review] PR with fix of showing parts of prev and next image in landscape mode Hi John The attached PR fixes the issue by using resizeHandler to call setFramesPosition. I have tested the fix on Hamachi device but am not able to test in on tablet. Please review and let me know if you see any changes. Thanks
Punam, There is a bug 958197 which is related to this one. This issue is caused by the enablement of APZ. If you turned that off, you will not have this issue. I will view this patch as a work around for that bug. Two information in your PR and comment here: 1. two resize events => this is also caused by APZ. See the test app in bug 961636. 2. tablet issue, you may use Firefox Nightly to open your app and resize the screen to 1280x800 (if you have phone UI around your main screen, you need to use 1312x861). And you can test your patch in tablet.
Comment on attachment 8363796 [details] [review] PR with fix of showing parts of prev and next image in landscape mode Thanks Punam, I had saw the PR. I don't think we need to have the patches of other bugs inside this one, maybe you don't know those bugs before. So, I just clear the flag. When you finish it, please put the review flag again.
Comment on attachment 8363796 [details] [review] PR with fix of showing parts of prev and next image in landscape mode Thanks John for feedback and groundwork you have done in finding root cause of this issue. I agree we can use this patch as a work around. I have updated the patch with your feedback. Please review
Comment on attachment 8363796 [details] [review] PR with fix of showing parts of prev and next image in landscape mode Punam, Thanks for finding the reason of unexpected error message. I prefer we write it in the comment and please add the bug number of APZ to comment if it is a work around.
Attachment #8363796 - Flags: review?(johu) → review+
Thanks John for the review, I have updated the PR with the comment. For some reason I am not seeing the merge button. Can you please help land the PR to master. Thanks
merged to master: https://github.com/punamdahiya/gaia/commit/fba3615444762a62a9b8d2d9800131dcb82614bd
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.