Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/mobile/app: android:screenOrientation is not honored #10943

Closed
rakyll opened this issue May 24, 2015 · 5 comments
Closed

x/mobile/app: android:screenOrientation is not honored #10943

rakyll opened this issue May 24, 2015 · 5 comments
Labels
FrozenDueToAge
Milestone
Unreleased

Comments

@rakyll
Copy link
Contributor

rakyll commented May 24, 2015

I have an application with the following AndroidManifest. android:screenOrientation="landscape" is ignored. The application starts in the portrait mode and screen orientation changes as I rotate the device. This value must have been handled during the EGLSurface creation and configuration changes on Android.

cc/ @crawshaw

<?xml version="1.0" encoding="utf-8"?>
<manifest
  xmlns:android="http://schemas.android.com/apk/res/android"
  package="org.golang.todo.main"
  android:versionCode="1"
  android:versionName="1.0">

  <uses-sdk android:minSdkVersion="9" />
  <application android:label="Drum machine" android:hasCode="false" android:debuggable="true">
  <activity android:name="android.app.NativeActivity"
    android:label="Drums"
    android:configChanges="orientation|screenSize"
    android:screenOrientation="landscape">
    <meta-data android:name="android.app.lib_name" android:value="drummachine" />
    <intent-filter>
      <action android:name="android.intent.action.MAIN" />
      <category android:name="android.intent.category.LAUNCHER" />
    </intent-filter>
  </activity>
  </application>
</manifest>
@rakyll rakyll mentioned this issue May 25, 2015
Open
@ianlancetaylor ianlancetaylor added this to the Unreleased milestone Jun 3, 2015
@crawshaw
Copy link
Member

crawshaw commented Jun 3, 2015

I have had a change for this sitting around for a while, but I'm working on proper testing first.

@gopherbot
Copy link

CL https://golang.org/cl/16150 mentions this issue.

@dskinner
Copy link
Member

@crawshaw while I submitted CL 16150, I don't consider it complete (I think there's a tag the Go team uses for this but I don't recall, this is my first CL). Specifically, testing needs a definitive target and not just whatever's installed on a developers machine as in the CL.

The test should enforce use of a specific build-tool version as a method of declaring the cmd's intended output compatibility.

I considered the current test broken but there are useful bits that should remain describing the format. The comments of skipByte for example should be retained somehow though the actual map is no longer necessary.

As a frame of reference, I have the following build tool versions installed locally and did a byte compare of manifest output:

19.1.0  21.1.1  false
19.1.0  21.1.2  false
19.1.0  22.0.1  false
19.1.0  23.0.1  false
21.1.1  21.1.2  true
21.1.1  22.0.1  true
21.1.1  23.0.1  true
21.1.2  22.0.1  true
21.1.2  23.0.1  true
22.0.1  23.0.1  true

All resource identifiers that can not be extracted from gomobile's minimum target (api-9?) should be excluded currently. This concern is a separate issue with gomobile since resource identifiers are hard-coded but contain no information wrt the api level the identifier was introduced. This may lead to invalid manifests containing resource identifiers for an api level higher than the declared target api of a developers manifest. I'm unaware of the defined behaviour in such a case.

This should not affect screenOrientation but I will double check each identifier that I did add to see if they come after the minimum target, pending discussion.

@crawshaw
Copy link
Member

@dskinner while I'm sure we could do more, your CL is already a huge improvement. Thank you!

@dskinner
Copy link
Member

Ok, I think some of those issues I mentioned should maybe get their own issue for tracking then as they have some unanswered questions that would only delay this CL for little reason. I'll follow up with that now.

@golang golang locked and limited conversation to collaborators Oct 24, 2016
imWildCat pushed a commit to imWildCat/go-mobile that referenced this issue Apr 10, 2021
@dskinner @hyangah
cmd/gomobile: support manifest screenOrientation
cf95a41 
Testing BinaryXML for the change was non-trivial as the first 4 bytes
of gomobile output did not match the latest version of aapt's output.
A new approach to testing compatibility was added based on how the
aapt tool pretty prints xmltree of manifest's contents.

Fixes golang/go#10943

Change-Id: I5c60af10931d9693dbeaff66f23a69042a78e8fa
Reviewed-on: https://go-review.googlesource.com/16150
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
imWildCat pushed a commit to imWildCat/go-mobile that referenced this issue Apr 11, 2021
@dskinner @hyangah
cmd/gomobile: support manifest screenOrientation
6b33ade 
Testing BinaryXML for the change was non-trivial as the first 4 bytes
of gomobile output did not match the latest version of aapt's output.
A new approach to testing compatibility was added based on how the
aapt tool pretty prints xmltree of manifest's contents.

Fixes golang/go#10943

Change-Id: I5c60af10931d9693dbeaff66f23a69042a78e8fa
Reviewed-on: https://go-review.googlesource.com/16150
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@dskinner @rakyll @crawshaw @ianlancetaylor @gopherbot