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

Android: bug(s) in runtime permissions #5205

Closed
pazos opened this issue Aug 13, 2019 · 9 comments
Closed

Android: bug(s) in runtime permissions #5205

pazos opened this issue Aug 13, 2019 · 9 comments

Comments

@pazos
Copy link
Member

pazos commented Aug 13, 2019

  • KOReader version: from 2019.07.
  • Device: Android (api23+)

Issue

  1. If the user don't give write permission on the external storage the activity closes itself but the native thread is still on the background uncompressing assets, until it fails when it can't write to cache folder solved

  2. If the user uninstalls the program but keeps /sdcard/koreader and had setup a custom timeout the next time the program is installed it will request the permission at launch (because it is stored on our own settings and restored every launch). It should request WRITE_SETTINGS just if the user does an explicit action (ie: changes screen brightness or screen timeout). solved

  3. Minor: the frontlight widget sets its new value before asking for permission.

@pazos
Copy link
Member Author

pazos commented Aug 29, 2019

Regarding:

Minor: the frontlight widget sets its new value before asking for permission

I think we would want to avoid touching system-wide light settings and just modify the light of our activity. In this case we don't need to ask permissions and we don't force the user to manual light levels outside KOReader.

@Frenzie
Copy link
Member

Frenzie commented Aug 29, 2019

Fully on board with not messing with system-wide light settings. :-)

@pazos
Copy link
Member Author

pazos commented Aug 29, 2019

Fully on board with not messing with system-wide light settings. :-)

Ok, I will implement this on top of koreader/android-luajit-launcher#171 to avoid ugly casting.

AFAICT the in-activity brightness is one way only (set a float between 0.0 and 1.0 to use custom levels and anything lower than 0 to use system defaults), which means there isn't any way to retrieve current level of the activity (as it is expected to be equal to system brightness or managed/recorded by the activity).

@pazos
Copy link
Member Author

pazos commented Aug 31, 2019

Implemented in koreader/android-luajit-launcher@7fd77e9.

I kept everything as it was. We get and set int values from MIN(0) to MAX(255). All these values are custom Any number below 0 will be treated as system defaults. For convenience BRIGHTNESS_DEFAULT(-1) will be used if the user doesn't override this setting.

Note 1: we start with default level and we need set custom user level, if exists, during device initialization. Custom level will be used only for this app.

Note 2: once a custom level is set changing screen brightness from android settings won't change the value in KOReader.

@pazos
Copy link
Member Author

pazos commented Aug 31, 2019

And now my headache begins:

I spent a few minutes reading frontend/ui/widget/frontlightwidget.lua for the first time and I would need some help here.

I would want to add a checkbox and differenciate between system and custom settings.

if android.getScreenTimeout() == -1 then
    -- use system settings, slider disabled and checkbox checked
else
    -- use custom settings, slider enabled and checkbox unchecked
end

@Frenzie
Copy link
Member

Frenzie commented Sep 1, 2019

@pazos I don't know, did @robert00s write it? You'd have to check git blame.

@robert00s
Copy link
Contributor

@pazos
I can help. Where do you want to put checkbox?

@pazos
Copy link
Member Author

pazos commented Sep 1, 2019

@robert00s: not sure exactly where it makes sense, but for now something like:

Sin nombre could be fine.

I'm not sure if add the checkbox logic only if Device:isAndroid() or add something new like Device:hasLightLevelFallback() which is only true on Android.

@robert00s
Copy link
Contributor

@pazos Done in #5307

Frenzie pushed a commit that referenced this issue Sep 4, 2019
See: #5205 (comment)

Devices with `hasLightLevelFallback = true` (for now Android) has extra checkbutton `Use system settings`. Default unchecked.
@Frenzie Frenzie closed this as completed Sep 4, 2019
mwoz123 pushed a commit to mwoz123/koreader that referenced this issue Mar 29, 2020
See: koreader#5205 (comment)

Devices with `hasLightLevelFallback = true` (for now Android) has extra checkbutton `Use system settings`. Default unchecked.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants