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

@empty item considered @null #1270

Closed
phhusson opened this Issue Jun 10, 2016 · 5 comments

Comments

Projects
None yet
3 participants
@phhusson
Contributor

phhusson commented Jun 10, 2016

This happens on apktool 2.1.1 running on Linux, APK coming from a Mediatek 6.0 ROM, but is most probably not MTK specific.

For instance this line:
https://android.googlesource.com/platform/frameworks/base/+/refs/heads/master/core/res/res/values/styles_material.xml#846
This contains a @empty
Current apktool decodes it to @null which is wrong, the behaviour is different.
For instance https://android.googlesource.com/platform/frameworks/base.git/+/master/core/java/android/widget/PopupWindow.java#231
The hasValueOrEmpty will return false for @null, but true for @empty, so apktool d/b will change the behaviour of the matching firmware.
Namely, the CTS android.widget.cts.ListPopupWindowTest#testAccessAnimationStyle fails without this change.

@iBotPeaches iBotPeaches added this to the 2.1.2 milestone Jun 10, 2016

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Jun 10, 2016

Owner

Very nice catch. This seems to be the underlying issue (I think) affecting #1116

My check for empty vs null must be wrong - https://github.com/iBotPeaches/Apktool/blob/master/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/data/value/ResValueFactory.java#L36

Owner

iBotPeaches commented Jun 10, 2016

Very nice catch. This seems to be the underlying issue (I think) affecting #1116

My check for empty vs null must be wrong - https://github.com/iBotPeaches/Apktool/blob/master/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/data/value/ResValueFactory.java#L36

@phhusson

This comment has been minimized.

Show comment
Hide comment
@phhusson

phhusson Jun 13, 2016

Contributor

Here is the fix I currently am using:
phhusson@a210eaf

Contributor

phhusson commented Jun 13, 2016

Here is the fix I currently am using:
phhusson@a210eaf

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Jun 13, 2016

Owner

Thanks! Looks good. I'll whip up some tests when I get a chance and get this merged in.

Owner

iBotPeaches commented Jun 13, 2016

Thanks! Looks good. I'll whip up some tests when I get a chance and get this merged in.

@ingbrzy

This comment has been minimized.

Show comment
Hide comment
@ingbrzy

ingbrzy Jun 13, 2016

@phhusson could you post your built apktool with that fix?

ingbrzy commented Jun 13, 2016

@phhusson could you post your built apktool with that fix?

@phhusson

This comment has been minimized.

Show comment
Hide comment

iBotPeaches added a commit that referenced this issue Jun 13, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment