Closed Bug 938312 Opened 10 years ago Closed 9 years ago

Wrong orientation when starting browser in landscape mode

Categories

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

Product:
Firefox OS Graveyard
Component:
Gaia::System::Window Mgmt
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
1.3 Sprint 6 - 12/6

People

(Reporter: milan, Assigned: vingtetun)

References

Details

(Keywords: regression)

Attachments

(3 files, 4 obsolete files)

What the bug looks like
1.90 MB, image/jpeg
Details
bug938312.gaia.patch
1.15 KB, patch
alive
: review+
Details | Diff | Splinter Review
bug938312.gecko.patch
1.17 KB, patch
kats
: review+
Details | Diff | Splinter Review
Attached image What the bug looks like — 
Go to the browser and open a page.
Go back to the home screen or lock screen.
Turn the phone to landscape.
Go to the browser.

The page size is as if the phone was on portrait orientation.  Scrolling works correctly.  If you rotate to portrait and back to landscape, the page size is correctly reset.

The attached image is from Unagi.  It happened on Hamachi as well, but there may be a timing issue, as it seems to be an intermittent problem on Hamachi, while it's 100% reproducible on Unagi.
blocking-b2g: --- → 1.3?
Keywords: regression, regressionwindow-wanted
Hi Milan, 

Could you please provide the build information for when you were able to reproduce this issue? I'm unable to reproduce this on Buri 1.3 Build ID: 20131113040200. If we could get some environmental variables, then the regression window can be found quicker. 

Gaia   7243b75041c93bc7deb240131748d0b164f3f0b0
SourceStamp 7b014f0f3b03
BuildID 20131113040200
Version 28.0a1

Thanks!
Flags: needinfo?(milan)
Is this site (http://msujaws.wordpress.com/2013/11/13/this-friday-computer-science-as-a-profession/) the target site you are using to this this?
(In reply to Jason Smith [:jsmith] from comment #2)
> Is this site
> (http://msujaws.wordpress.com/2013/11/13/this-friday-computer-science-as-a-
> profession/) the target site you are using to this this?

to reproduce this*
It was planet.mozilla.org.  Botond, you were talking about a bug you were looking at, and it sounded like a duplicate of this.  Can you take a look at the symptoms here and let us know?
Flags: needinfo?(milan) → needinfo?(botond)
(In reply to Sarah Parsons from comment #1)
> Hi Milan, 
> 
> Could you please provide the build information for when you were able to
> reproduce this issue? I'm unable to reproduce this on Buri 1.3 Build ID:
> 20131113040200. If we could get some environmental variables, then the
> regression window can be found quicker. 
> 
> Gaia   7243b75041c93bc7deb240131748d0b164f3f0b0
> SourceStamp 7b014f0f3b03
> BuildID 20131113040200
> Version 28.0a1
> 
> Thanks!

This is a 26.0 build was done locally on 20131112093756:
Gecko d5971002aa9ee1f079a655898ec5671ac9f6d0cd
Gaia f62feb9f981df01a7e2aab2dc6aaa0a8dc1eea17
I can't reproduce this on my Hamachi device. BenWa also couldn't reproduce it again on his device when we tried. I can do a Unagi build to see if I can reproduce it there.

It's possible that my patch for bug 937896 fixes this, but I won't know until I try.
Flags: needinfo?(botond)
These are the detailed steps:
0 Go to planet.mozilla.org and scroll around a bit.  Then:

1 Home screen, in portrait
2 Open browser
3 Close browser
4 Phone to landscape
5 Open browser
I wasn't able to reproduce this with a Unagi build of master, either (though, granted, I used a different website as I wasn't able to get my phone to load planet.mozilla.org for some reason).

Is it possible that the issue was fixed in between 26 and master?
I was able to get this to reproduce on the 8/01/13 1.2 build and later, HOWEVER most of the time it would only occur for a few seconds while the website loaded. Once it completely loaded, the browser would fix itself and display correctly in landscape without me intervening in any way. 

The regression window below may not be accurate since reproducing this issue was fairly difficult for me and it could very well happen in earlier builds, but I feel this is pretty close. Also, the 7/31/13 1.2 build had an issue where the web page would not display at all when going back into the Browser with the phone rotated to landscape, hence the window being 7/30 to 8/01.

- Works -
Environmental Variables:
Device: Buri v1.2 MOZ RIL
BuildID: 20130730030200
Gaia: ba5ff211fbf6a930326cc6a0d4a1205a7528630b
Gecko: 3d40d270c031
Version: 25.0a1
Firmware Version: 20131104

- Broken -
Environmental Variables:
Device: Buri v1.2 MOZ RIL
BuildID: 20130801030224
Gaia: c1620a242c937c7fcb171b88c85ef63561165081
Gecko: 05d3797276d3
Version: 25.0a1
Firmware Version: 20131104
Keywords: regressionwindow-wanted
QA Contact: mvaughan
comment 9 makes me think that this is a regression, but it's a minor one. If this is difficult to reproduce & mostly fixes itself when the website is loaded, I'm inclined to indicate that this isn't a blocker.
I can live with that, it has a simple workaround.
blocking-b2g: 1.3? → -
I can reproduce the issue with some landscape app, like poppit. Opening the app the first time is fine, then hit home, open the app again and the display is broken. All this happens on a Keon but is suspect that some other devices are broken as well.
blocking-b2g: - → 1.3?
Attached patch bug938312.workaround.patch (obsolete) — — Splinter Review 
I have no idea what's going on exactly, but this workaround seems to fix the issue for poppit for me.

I have look a bit inside the resize method of $GAIA/apps/system/js/window.js and the size of the iframe is correct while it is rendered the wrong size on the screen.
Seems like the workaround breaks a bit the attention screen!
(In reply to Vivien Nicolas (:vingtetun) (:21) (RTO - Review Time Off until 09/12/13) from comment #14)
> Seems like the workaround breaks a bit the attention screen!

Maybe not. Someone turns on an option in my pref that change a few thing about window sizing. Turning it back I can't reproduce the issue I was seeing.
Component: Gaia::Browser → Gaia::System::Window Mgmt
Attached patch bug938312.patch (obsolete) — — Splinter Review 
This solve part of the issue. It basically solve the issue for 1.3 without APZ turned on. With APZ turned on there is some weird zooming issue since the app has been designed to run in landscape and we kind of force it to display in landscape with the sleep menu or the attention screen.

This create 2 issues. One if that this is ugly. The other is that if change the width of the content and it ends up in a zoom at some point. This is still unclear to me why we are so zoomed. A possible solution would simply be to hide the app when the sleep menu is asked, when there is an attention screen opened, or when the phone is locked. Not the best UX but man, it won't be as ugly as the current situation.

I may do that in a different patch.
This Gaia patch prevent landscape app to be resized with portrait dimensions.
Attachment #8340424 - Attachment is obsolete: true
Attachment #8341901 - Attachment is obsolete: true
This Gecko patch prevent the intrinsic scale to be smaller than min-zoom/ or bigger than max-zoom.
Comment on attachment 8342278 [details] [diff] [review]
gaia.prevent.landscape.to.be.portrait.patch

Alive, what are the bad things that can happens if we do that?

Also we only need the Gaia patch, there is no need for the Gecko patch if we do that.
Attachment #8342278 - Flags: review?(alive)
OS: FreeBSD → Gonk (Firefox OS)
Hardware: x86 → ARM
I'm not sure what this is going to fix.
Also I cannot reproduce on my unagi even with APZ enabled.
But I found there's something wrong:

1. Open browser, switch to landscape
2. Click home button
3. Rotate to portrait
4. Open browser again

You would see it's still in landscape.

Is this the same thing as the bug?
If so I think I'd fixed it in bug 907013 but not in this way.

The way seems too strange: orientationchange would trigger resize event on window of system app and window manager would resize the current active app. If you see the size is incorret it must be appWindow.resize() is not called correctly.
Comment on attachment 8342278 [details] [diff] [review]
gaia.prevent.landscape.to.be.portrait.patch

Review of attachment 8342278 [details] [diff] [review]:
-----------------------------------------------------------------

Anyway you shouldn't hardcode 'landscape' or this may break tablet!
Read OrientationTable part in appWindow.
Attachment #8342278 - Flags: review?(alive)
(In reply to Alive Kuo [:alive][NEEDINFO] from comment #20)
> I'm not sure what this is going to fix.

It fix what you see in comment #1. And a zoom issue with APZ enabled.

> Also I cannot reproduce on my unagi even with APZ enabled.

Not sure why you can't see it.

> But I found there's something wrong:
> 
> 1. Open browser, switch to landscape
> 2. Click home button
> 3. Rotate to portrait
> 4. Open browser again
> 
> You would see it's still in landscape.
> 
> Is this the same thing as the bug?

That's a different issue. My patch should not touch that, so I guess that you can reproduce without the patch, right?
 
> The way seems too strange: orientationchange would trigger resize event on
> window of system app and window manager would resize the current active app.
> If you see the size is incorret it must be appWindow.resize() is not called
> correctly.

The size is correct, but the app has not been written to support landscape/portrait switching and weird thing happens. I still think we should not resize the app but maybe there is an underlying Gecko issue here. Will still investigate.
Seems like we are lying to the child and tell it incorrect dimensions.
(In reply to Vivien Nicolas (:vingtetun) (:21) (RTO - Review Time Off until 09/12/13) from comment #23)
> Seems like we are lying to the child and tell it incorrect dimensions.

Fun! When we tell him the righ dimensions that where it starts to overzoom.
Alive, this resize has been introduced during your change to the orientation bug I'm not sure what purpose it deserved since app.resize is called during app opening.

Removing it will prevent us to lie to Gecko about the frame dimensions.
Attachment #8342278 - Attachment is obsolete: true
Attachment #8342609 - Flags: review?(alive)
Kats, the change to UpdateCompositionBounds has been introduced in https://bugzilla.mozilla.org/show_bug.cgi?id=923431. It seems like it does not respect the min-zoom/max-zoom define by the app. (I first thought the issue was in HandlePossibleViewportChange but I was shooting at the wrong bird!).

With this patch, we stop to overzoom the frame when changing orientation.
Attachment #8342279 - Attachment is obsolete: true
Attachment #8342611 - Flags: review?(bugmail.mozilla)
(In reply to Vivien Nicolas (:vingtetun) (:21) (RTO - Review Time Off until 09/12/13) from comment #26)
> Created attachment 8342611 [details] [diff] [review]
> bug938312.gecko.patch
> 
> Kats, the change to UpdateCompositionBounds has been introduced in
> https://bugzilla.mozilla.org/show_bug.cgi?id=923431. It seems like it does
> not respect the min-zoom/max-zoom define by the app.

Which change are you referring to? As far as I can see, the only change bug 923431 made to UpdateCompositionBounds is a change to how the strongly typed units are used, no actual change to the calculation.
Attachment #8342611 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8342609 [details] [diff] [review]
bug938312.gaia.patch

Review of attachment 8342609 [details] [diff] [review]:
-----------------------------------------------------------------

I would track this in my bug as well.
Attachment #8342609 - Flags: review?(alive) → review+
https://github.com/mozilla-b2g/gaia/commit/bf26668567e91ac414b22612bf9a4be2359416fb

Gaia commit. I waiting for the try server build to finished before landing the Gecko one.
(In reply to Vivien Nicolas (:vingtetun) (:21) (RTO - Review Time Off until 09/12/13) from comment #30)
> https://github.com/mozilla-b2g/gaia/commit/
> bf26668567e91ac414b22612bf9a4be2359416fb
> 
> Gaia commit. I waiting for the try server build to finished before landing
> the Gecko one.

Seems green. https://tbpl.mozilla.org/?tree=Try&rev=8ae2ce02ae49
https://hg.mozilla.org/integration/mozilla-inbound/rev/6074cfb9ee9c
Assignee: nobody → 21
Status: NEW → ASSIGNED
Target Milestone: --- → 1.3 Sprint 6 - 12/6
https://hg.mozilla.org/mozilla-central/rev/6074cfb9ee9c
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Already in 1.3, so clearing nom.
blocking-b2g: 1.3? → ---
You need to log in before you can comment on or make changes to this bug.