We would like to add a new keyword 'default' to the orientation API. The 'default' keyword is either mapped to 'portrait-primary' or 'landscape-primary', it will depend on the device. For example, if the panel resolution is 1280*800, 'default' will make it landscape, if the resolution is 800*1280, 'default' will make it to portrait. Now we have to consider the cases in 1) orientation in manifest 2) screen.lockOrientation It should make System app of Gaia working on the portrait phone and landscape tablet in same code.

We need API to know default orientation means landscape or portrait. Because system app needs to rotate the window during opening and closing transition, and "default" makes us don't know which degree to rotate. This needs gaia side fix otherwise the transition would fail.

We add 'default' keyword mapped to either 'portrait-primary' or 'landscape-primary' in OrientationObserver base on width and height of screen.

add comments on mozLockOrientation in webidl, it can be landscape-primary, landscape-secondary, landscape, portrait-primary, portrait-secondary, portrait, or default.

Comment on attachment 797154 [details] [diff] [review] add comments on mozLockOrientation Review of attachment 797154 [details] [diff] [review]: ----------------------------------------------------------------- Mounir has owned this API so far

Comment on attachment 797151 [details] [diff] [review] add orientation keyword 'default' to express normal orientation Review of attachment 797151 [details] [diff] [review]: ----------------------------------------------------------------- Instead of using this ugly global variable, why don't use lock the screen to DEFAULT and then the hal backend we decide what DEFAULT means. Then, nsScreen.cpp will get an event about the new screen orientation. No need for the DOM to know about what DEFAULT means. ::: dom/base/ScreenOrientation.h @@ +16,5 @@ > static const ScreenOrientation eScreenOrientation_PortraitPrimary = 1u << 0; > static const ScreenOrientation eScreenOrientation_PortraitSecondary = 1u << 1; > static const ScreenOrientation eScreenOrientation_LandscapePrimary = 1u << 2; > static const ScreenOrientation eScreenOrientation_LandscapeSecondary = 1u << 3; > +static const ScreenOrientation eScreenOrientation_default = 1u << 4; nit: style ::: dom/base/nsScreen.cpp @@ +330,5 @@ > orientation |= eScreenOrientation_LandscapePrimary; > } else if (item.EqualsLiteral("landscape-secondary")) { > orientation |= eScreenOrientation_LandscapeSecondary; > + } else if (item.EqualsLiteral("default")) { > + orientation |=eScreenOrientation_default; nit: style ::: widget/gonk/OrientationObserver.cpp @@ +27,5 @@ > > using namespace mozilla; > using namespace dom; > > +extern bool naturallyPortrait; Why are you using a global variable here?

(In reply to Mounir Lamouri (:mounir) from comment #5) > ::: widget/gonk/OrientationObserver.cpp > @@ +27,5 @@ > > > > using namespace mozilla; > > using namespace dom; > > > > +extern bool naturallyPortrait; > > Why are you using a global variable here? Thank you for your feedback! My original thought is set the 'default' in nsScreen::MozLockOrientation() and check what the 'default' means in OrientationObserver::LockScreenOrientation(). I'm trying to get the panel resolution in OrientationObserver::LockScreenOrientation(). Firstly, I would like to used the API we have now as below: nsCOMPtr<nsIScreen> screen = GetPrimaryScreen(); screen->GetRect(&left, &top, &width, &height) However, the width and height is variant since the 'sVirtualBounds' shows the information about the current windows, like 1280*800 in landscape and 800*1280 in portrait. So we should use the information of width and height from 'gScreenBounds' which is static. We can not access 'gScreenBounds' here, that's why I choose a global variable to save the panel is portrait or landscape even though it is ugly.

Hi Mounir, I think we can used the information we saved in DetectDefaultOrientation(). When we need to LockScreenOrientation to 'default', we can set it as portrait or landscape directly. Thank you!

Comment on attachment 801492 [details] [diff] [review] add orientation keyword 'default' to express normal orientation (v2) Review of attachment 801492 [details] [diff] [review]: ----------------------------------------------------------------- We need every backend to be updated before landing this. ::: dom/base/ScreenOrientation.h @@ +16,5 @@ > static const ScreenOrientation eScreenOrientation_PortraitPrimary = 1u << 0; > static const ScreenOrientation eScreenOrientation_PortraitSecondary = 1u << 1; > static const ScreenOrientation eScreenOrientation_LandscapePrimary = 1u << 2; > static const ScreenOrientation eScreenOrientation_LandscapeSecondary = 1u << 3; > +static const ScreenOrientation eScreenOrientation_default = 1u << 4; nit: eScreenOrientation_Default ::: widget/gonk/OrientationObserver.cpp @@ +267,5 @@ > + eScreenOrientation_default)); > + > + if (aOrientation == eScreenOrientation_default) { > + aOrientation = (sOrientationOffset == sDefaultPortrait) ? eScreenOrientation_PortraitPrimary : > + eScreenOrientation_LandscapePrimary; nit: indentation seems pretty messy around here.

Hi Mounir, Could you please help to share more about the backend in your suggestion? My original thought is to enable this 'default' orientation, and then Gaia team can start to move some of their setting to 'default'. I'm not so sure about what the backend we should check. Thank you!

Comment on attachment 802193 [details] [diff] [review] add orientation keyword 'default' to express normal orientation (v3) Review of attachment 802193 [details] [diff] [review]: ----------------------------------------------------------------- There are other backends for the Screen Orientation API. See the code in hal/. For every backend, we should support this new keyword, otherwise we can't ship it.

Hi Mounir, I think android is the only backend we need to consider besides b2g. I follow the instructions below to build android application. https://wiki.mozilla.org/Mobile/Fennec/Android#Linux I'm trying to add android:screenOrientation in mobile/android/base/AndroidManifest.xml.in to check the orientation behavior. I expect we can used same attribute (landscape-primary, portrait-primary...) like we used in b2g. However, I can not use it by adding android:screenOrientation="landscape-primary" in xml file. I'm thinking about android is using standard api should not include MozLockOrientation. I would like to write a test app and use Fennec on android to verify.

Support gonk/android for this new orientation keyword 'default'

Comment on attachment 809061 [details] [diff] [review] add orientation keyword 'default' to express normal orientation (v4) Review of attachment 809061 [details] [diff] [review]: ----------------------------------------------------------------- Brad, could you review the Android changes?

Comment on attachment 809061 [details] [diff] [review] add orientation keyword 'default' to express normal orientation (v4) Review of attachment 809061 [details] [diff] [review]: ----------------------------------------------------------------- r=blassey assuming my assumption is correct, r- othewise ::: dom/base/ScreenOrientation.h @@ +16,5 @@ > static const ScreenOrientation eScreenOrientation_PortraitPrimary = 1u << 0; > static const ScreenOrientation eScreenOrientation_PortraitSecondary = 1u << 1; > static const ScreenOrientation eScreenOrientation_LandscapePrimary = 1u << 2; > static const ScreenOrientation eScreenOrientation_LandscapeSecondary = 1u << 3; > +static const ScreenOrientation eScreenOrientation_Default = 1u << 4; it might be good to add a comment as to what "default" means. I'm taking it to mean that we're ignoring the sensor, is that correct?

Hi Brad, the "default" means either "portrait-primary" or "landscape-primary", it will depend on the display resolution of devices. If display resolution is 1280*800, "default" means landscape, if the resolution is 800*1280, "default" means portrait. For android backend, I use SCREEN_ORIENTATION_NOSENSOR directly. It should be same as our expect. Thank you for your suggestion!

Comment on attachment 810420 [details] [diff] [review] Bug 908058 - add orientation keyword 'default' to express normal orientation (v5) Review of attachment 810420 [details] [diff] [review]: ----------------------------------------------------------------- thanks, comment makes it clearer

add reviewer in comment.

Doc updated: https://developer.mozilla.org/en-US/docs/Web/API/Screen.lockOrientation and https://developer.mozilla.org/en-US/Firefox/Releases/26