Implement the orientation property for web apps on Android. See https://developer.mozilla.org/en/Apps/Manifest for a description of what the orientation property does. Note - Current docs (as of the time of this bug filing) are slightly off - we interpret these properties: - portrait-primary - landscape-primary - portrait-secondary - landscape-secondary

Note that the value is a comma-separated list. We should support the following: - portrait-primary Locked to a single portrait direction. If the device has an obvious primary orientation in portrait mode, this is that orientation. If there is no obvious primary orientation due to landscape being the primary orientation for the device, this is the orientation if rotating the device 90 degrees clock-wise from the primary landscape mode. - landscape-primary Locked to a single landscape direction. If the device has an obvious primary orientation in landscape mode, this is that orientation. If there is no obvious landscape orientation due to portrait being the primary orientation for the device, this is the orientation if rotating the device 90 degrees clock-wise from the primary portrait mode. - portrait-secondary The portrait mode opposite of "portrait-primary". - landscape-secondary The landscape mode opposite of "landscape-primary". - portrait Equivalent to "portrait-primary,portrait-secondary". - landscape Equivalent to "landscape-primary,landscape-secondary".

I think I am going to set these as a "default" state. If the app changes them using DOM Api's, I'll honor the new settings. If they then unlock using DOM API's, we'll switch back to the manifest values? Flash fullscreen also locks the orientation, but I figure we can do the same thing? Honor Flash. Revert back when it unlocks? Sound ok?

That sounds awesome!

Jonas - Do you know of a sample app that works on Firefox OS using the screen orientation manifest properties?

Unfortunately not.

We have to rewrite our orientation implementation on mobile for this to work. The current spec allows you to specify something like primary_portrait,secondary_landscape and allow only those two orientations. AFAICT, Android doesn't allow that, so I'm trying to listen to rotation changes here, figure out what orientation they correspond to, and adjust. Android doesn't seem to provide much info on what orientation a particular rotation value corresponds to either. I'm trying to use getRotation along with getConfiguration().orientation to determine the default state of the device. Seems to work, kinda sorta sometimes on the devices I have here, but I think I probably need something smarter.... or maybe we just say "Android won't let us do that" for now. Feedback/ideas blassey?

I'm totally fine with not having perfect support on Android for now. The only real-world app behavior that I've seen that would require a comma separated list is apps that want to allow all orientations except upside-down, so something like "primary-portrait,landscape". I think it would be ok behavior to simply ignore the orientation manifest property if we run into something like that for now.

This allows setting the default orientation, and creates a pref, read at startup, to set it.

This gives us the ability to write a defaultprefs.js file on install of the app. Those prefs are set on first run. This relies on the patch in bug 769821. App manifests don't actually have support for orientation yet. I'm assuming here that we'll just get back a string from them...? That's simple and will be part 3 of this patch. But maybe we'd rather do the parsing in Gecko?

Grr. Forgot to qref.

This is the simplest way to add orientation support (I think?)

Comment on attachment 647709 [details] [diff] [review] Patch 3/3 - Webapps.jsm changes Looks good to me. Simple change, I don't think it needs extra review from someone from dom

blassey, I want to point out that I didn't fix our orientation implementation here. I can file a follow up for that, but didn't want it to hold up webapps stuff.

Comment on attachment 647707 [details] [diff] [review] Patch >diff --git a/mobile/android/chrome/content/WebAppRT.js b/mobile/android/chrome/content/WebAppRT.js >+ defaultPrefs = WebappsUI.readDefaultPrefs(FileUtils.getFile("ProfD", [WebappsUI.DEFAULT_PREFS_FILENAME])); Move "readDefaultPrefs" to WebAppRT since it's not used in WebappsUI Add a second const DEFAULT_PREFS_FILENAME in WebAppRT (I don't like to share) >diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js >+ DEFAULT_PREFS_FILENAME: "defaultprefs.js", default-prefs.js" > doInstall: function doInstall(aData) { >+ (function(icon) { > var profilePath = sendMessageToJava({ var -> let > var file = null; var -> let > if (profilePath) { > var file = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsILocalFile); "var" saved you here, but remove the "var" and don't redefine "file" >+ writeDefaultPrefs: function webapps_writeDefaultPrefs(aFile, aPrefs) { >+ var ostream = Cc["@mozilla.org/network/file-output-stream;1"].createInstance(Ci.nsIFileOutputStream); var -> let >+ readDefaultPrefs: function webapps_readDefaultPrefs(aFile) { >+ var prefsString = NetUtil.readInputStreamToString(fstream, fstream.available(), {}); var -> let

https://hg.mozilla.org/integration/mozilla-inbound/rev/27de96f5f67a https://hg.mozilla.org/integration/mozilla-inbound/rev/6dd4e5c2fe0a

Comment on attachment 647707 [details] [diff] [review] Patch [Approval Request Comment] Bug caused by (feature/regressing bug #): New Webapps stuff User impact if declined: Orientation isn't used, but we're also interested in using this functionality to send other information from Fennec to the apps (for instance, about whether to telemetry ping). Testing completed (on m-c, etc.): Landed on inbound 8/8 Risk to taking this patch (and alternatives if risky): Low risk. Webapp only String or UUID changes made by this patch: None.

Flagging qawanted for testing the current changes landing.

https://hg.mozilla.org/mozilla-central/rev/6dd4e5c2fe0a https://hg.mozilla.org/mozilla-central/rev/27de96f5f67a

This specifically doesn't effect fennec

Comment on attachment 647709 [details] [diff] [review] Patch 3/3 - Webapps.jsm changes If this is a blocking feature this stuff should move forward. If its not then its probably better to just hold it. But the increased testing on Aurora would be good for us. (note there's still more work to finish this feature) [Approval Request Comment] Bug caused by (feature/regressing bug #): New webapps feature User impact if declined: webapps asking for manifest.orientation will fail Testing completed (on m-c, etc.): landed on mc today Risk to taking this patch (and alternatives if risky): fairly low. worst case it should return "". String or UUID changes made by this patch: None

Comment on attachment 647707 [details] [diff] [review] Patch only affects webapps, approving for Aurora.

blassey ping? I know you said you had questions about one of my patches. Can you drop them in here?

Comment on attachment 647700 [details] [diff] [review] Path 1/3 Review of attachment 647700 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/GeckoScreenOrientationListener.java @@ +119,5 @@ > } > > + public void handleMessage(String event, JSONObject message) { > + try { > + if (event.equals("Preferences:Data")) { as discussed, this stupid. Let's fix it in a follow up. @@ +153,5 @@ > + return orientationFromString(orientations.get(0)); > + } > + > + private short orientationFromString(String val) { > + if (val.equals("portrait")) "portrait".equals(val) same for the rest

Note - I tried testing portrait as an app manifest property, but that did not work at all.

This has not all landed yet.

FYI to Aaron for testing: I recently did testing on this feature for Firefox OS in a session-based testing style. You can see the results of the testing I did here - https://wiki.mozilla.org/B2G/QA/Sessions/Screen_Orientation_App_Manifest_Property_%2808/21/2012%29. In short, the easiest way to test this is using testmanifest.com and creating an orientation property in the manifest with different orientation combinations.

This doesn't seem to be working at all for me with the supplied property values in the manifest. `{ "orientation" : "landscape" }` doesn't seem to work on my Galaxy Nexus — when I launch the application with my device in portrait position, the application launches in portrait positioning and is not locked on rotation. `{"orientation" : "portrait"}`as well doesn't seem to work on my Galaxy Nexus - when I launch the application with my device in landscape position, the application launches in landscape positioning and is not locked on rotation. The same both apply with `{"orientation" : "landscape-primary"}` and `{"orientation" : "portrait-primary"}`. Am I missing something here?

Comment on attachment 647700 [details] [diff] [review] Path 1/3 [Approval Request Comment] Bug caused by (feature/regressing bug #): New stuffbr User impact if declined: orientation in manifests don't work. Testing completed (on m-c, etc.): Landed today Risk to taking this patch (and alternatives if risky): Medium risk. Some changes to our orientation support but nothing should be changed. String or UUID changes made by this patch: None.

Comment on attachment 647700 [details] [diff] [review] Path 1/3 Approving for Aurora due to need for webapp testing.

(In reply to Aaron Train [:aaronmt] from comment #29) > This doesn't seem to be working at all for me with the supplied property > values in the manifest. > > `{ "orientation" : "landscape" }` doesn't seem to work on my Galaxy Nexus — > when I launch the application with my device in portrait position, the > application launches in portrait positioning and is not locked on rotation. > > `{"orientation" : "portrait"}`as well doesn't seem to work on my Galaxy > Nexus - when I launch the application with my device in landscape position, > the application launches in landscape positioning and is not locked on > rotation. > > The same both apply with `{"orientation" : "landscape-primary"}` and > `{"orientation" : "portrait-primary"}`. > > Am I missing something here? Hmm.. not sure. I tested by hardcoding a value, but I will look tomorrow.

https://hg.mozilla.org/releases/mozilla-aurora/rev/2ac946714323 https://hg.mozilla.org/releases/mozilla-aurora/rev/cdf595eaae64