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

[addons] add setting for enabling/disabling zip install #8857

Merged
merged 1 commit into from Jan 22, 2016

Conversation

tamland
Copy link
Member

@tamland tamland commented Jan 13, 2016

This changes zip install to be handled similar to how Android handles apk installing from unknown sources. I.e when attempting to install a zip, it will take you to the settings where unknown sources must be explicitly enabled.

@razzeee
Copy link
Member

razzeee commented Jan 13, 2016

Haven't checked the code yet, but I love the idea.

@ronie
Copy link
Member

ronie commented Jan 13, 2016

nice!


msgctxt "#36618"
msgid "Warning!"
msgstr ""

This comment was marked as spam.

@MartijnKaijser
Copy link
Member

Could you also add file location for the other used strings?

@da-anda
Copy link
Member

da-anda commented Jan 13, 2016

hmm, shouldn't we rather hide the "install from zip" entry if unknown sources is disabled?

@tamland
Copy link
Member Author

tamland commented Jan 13, 2016

hmm, shouldn't we rather hide the "install from zip" entry if unknown sources is disabled?

No. It's by design, otherwise there will be endless confusion about "where did install from zip go?" We can reevaluate in a release or two.

@da-anda
Copy link
Member

da-anda commented Jan 13, 2016

how does waiting for a release or two change anything about the confusion?

@MartijnKaijser
Copy link
Member

people will follow some shitty youtube video now or in two years so better go the proper way at once

@da-anda
Copy link
Member

da-anda commented Jan 13, 2016

I'd also like to avoid a "click, click, click click, click, done" situation which we would end up with now. Oh, btw, which button is focused in the "are you sure to enable" dialog? IMO it should be the "no" button so that the user is somewhat forced to read before simply clicking.

@zag2me
Copy link
Contributor

zag2me commented Jan 13, 2016

I use install from zip all the time in my testing and so do many others. It should stay visible imo to prevent confusion. It is also there in many wiki guides and online articles (not all piracy related).

I like this solution. The android system is nice and feels more secure.

Most of the piracy Add-ons are installed by adding a repository from the system >> file manager route currently.

@MartijnKaijser
Copy link
Member

No they also use install from zip but then from the just added source from file manager. I don't see the issue on hiding it. Once you unable "untrusted" you having it back. Wiki can be updated

@tamland
Copy link
Member Author

tamland commented Jan 13, 2016

Perhaps that was a poor explanation, as I didn't think I had to explain: The purpose of this is to convey a message to the users about the security implications before making the decision of installing an add-on. It's not about removing or hiding this feature. Thus, it's important that "Install from zip" stays visible, including the dialog explaining why it's disabled and where to change it.

There are equally many clicks when going via the side menu. And please, just test it. It will not select the "dangerous" options by default.

#. Text in confirm dialog when enabling setting "System -> Add-ons -> Unknown sources"
#: xbmc/addons/AddonSystemSettings.cpp
msgctxt "#36618"
msgid "Add-ons will be given access to personal data stored on this device. By allowing, you agree that you are solely responsible for any loss of data, unwanted behaviour, or damage to your device that may result from using these add-ons. Proceed?"

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@Hedda
Copy link
Contributor

Hedda commented Jan 14, 2016

Yes please consider including a more verbose warning text similar to Android, as discussed here:

http://forum.kodi.tv/showthread.php?tid=229648

Linked examples are description text that Amazon and Google use for Enable Unknown Sources:

http://www.amazon.com/gp/help/customer/display.html?nodeId=201482620

https://support.google.com/accounts/answer/2812853

http://developer.android.com/distribute/tools/open-distribution.html

Great work btw!

@tamland
Copy link
Member Author

tamland commented Jan 17, 2016

jenkins build this please

@tamland tamland added this to the Krypton 17.0-alpha1 milestone Jan 17, 2016
@tamland
Copy link
Member Author

tamland commented Jan 21, 2016

good to go?

@@ -17814,7 +17820,29 @@ msgctxt "#36614"
msgid "Show add-ons that are currently running in the background."
msgstr ""

#empty strings from id 36615 to 36899
#: system/settings/settings.xml

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

tamland added a commit that referenced this pull request Jan 22, 2016
[addons] add setting for enabling/disabling zip install
@tamland tamland merged commit e1637b4 into xbmc:master Jan 22, 2016
@razzeee razzeee added Type: Feature non-breaking change which adds functionality v17 Krypton Component: Add-ons labels Jan 22, 2016
@zag2me
Copy link
Contributor

zag2me commented Jan 30, 2016

I just tested this on the nightlies and the text needs to be made a lot shorter. At the moment it comes up completely incomprehensible as it starts at the end of the sentience and scrolls over 4 lines.

screenshot001

Suggestion: "It can be potentially dangerous to install Add-ons from outside the official Kodi repository. Would you still like to proceed?"

The feature works well otherwise. A nice addition.

@tamland
Copy link
Member Author

tamland commented Jan 30, 2016

No idea why it's so long for you. Looks like this here:
screenshot044

@tamland
Copy link
Member Author

tamland commented Jan 30, 2016

Nvm. my window's stretched a few pixels which somehow cause the font to be rendered slightly different (my bet's on our glyph positioning hacks) sigh

@da-anda
Copy link
Member

da-anda commented Jan 30, 2016

just make that dialog bigger. I'd prefer to not shorten it as it currently really scares people off by giving examples of what could happen, while with the version from Zag people just think "whatever" and continue without thinking about the risks themselves.

@razzeee
Copy link
Member

razzeee commented Jan 30, 2016

@da-anda is right I think

@ronie
Copy link
Member

ronie commented Jan 30, 2016

#8994

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Add-ons Type: Feature non-breaking change which adds functionality v17 Krypton
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants