Skip to content

Conversation

@jancborchardt
Copy link
Member

@jancborchardt jancborchardt commented Jun 11, 2016

@schiessle let’s get a work-in-progress pull request going here. ;)

I found 2 good color pickers:

Also todo:

  • add labels to fields
  • make sure color and logos are replaced everywhere possible (favicon etc)

@MariusBluem
Copy link
Member

Will we also integrate generating branded clients like it was possible with ownBrander?

@blizzz blizzz modified the milestones: Nextcloud 10, Nextcloud 9 Jun 12, 2016
@blizzz
Copy link
Member

blizzz commented Jun 12, 2016

PR against master → milestone 10. backport will be 9.

@MorrisJobke
Copy link
Member

Will we also integrate generating branded clients like it was possible with ownBrander?

Branding clients is a bit harder, because you need a huge toolchain to build it. So this is limited to the web UI for now.

@jancborchardt
Copy link
Member Author

@schiessle when rebasing this against master, I get an internal server error. Can you check this out and the color picker as well?

cc @MorrisJobke @LukasReschke for JS help maybe ;)

@schiessle
Copy link
Member

when rebasing this against master, I get an internal server error. Can you check this out and the color picker as well?

working at it at the moment. Seems to have more influences at the core as expected and also breaks initial installation at the moment. Let's see what i can do.

@schiessle schiessle force-pushed the theming-app branch 2 times, most recently from 2362856 to ef66221 Compare June 13, 2016 16:34
@schiessle
Copy link
Member

@jancborchardt please have a look. The basic functionality is now there but we can probably improve the visual presentation.

I don't really understand why I can't update the logo directly after it was uploaded: https://github.com/nextcloud/server/pull/59/files#diff-7336f3dab52c02ef94f856620590f56bR34 @blizzz do you have a idea why this doesn't work?

@blizzz
Copy link
Member

blizzz commented Jun 13, 2016

👎 remote code execution. You can upload a PHP file as icon (e.g. phpinfo.php) and open it via http://nextcloud.server/themes/theming-app/core/img/phpinfo.php

@blizzz
Copy link
Member

blizzz commented Jun 13, 2016

I don't really understand why I can't update the logo directly after it was uploaded: https://github.com/nextcloud/server/pull/59/files#diff-7336f3dab52c02ef94f856620590f56bR34 @blizzz do you have a idea why this doesn't work?

For one, you need to wrap the path into url("…"), for the other the path was wrong. I got it working with:

logos[i].style.background = "url('" + OC.getRootPath() + "/themes/theming-app/core/img/" + value + "')";

@schiessle
Copy link
Member

Thanks @blizzz, this works!

@LukasReschke What's the best way to prevent people from uploading PHP files als background image? Maybe we can also just block all img/ paths from execution?

*
*/

if(!file_exists(\OC::$SERVERROOT . '/themes/theming-app')) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't themes read-only by default?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I check it here https://github.com/nextcloud/server/pull/59/files#diff-d1ebf4e994bc54194a285284dda2224aR40

But good point to also add a check here to only enable the theme if we can write to the themes folder.

@schiessle schiessle force-pushed the theming-app branch 2 times, most recently from 94c1c71 to b230bfc Compare June 16, 2016 15:30
@LukasReschke LukasReschke self-assigned this Jun 16, 2016
@LukasReschke
Copy link
Member

As discussed with @schiessle I'll hijack this branch with some security hardenings for this feature.

@joergmschulz
Copy link

Do you already have some kind of specification document for theming? This is the area where I could help - if somebody gives me a hint for a starting point.

@schiessle
Copy link
Member

@joergmschulz great! 😄 Did you already saw our documentation repository? Would be great to have for the administration documentation a short description of the theming app, with a screenshot and so on: https://github.com/nextcloud/documentation/tree/master/admin_manual

Would be great if you could help with this!

@jancborchardt
Copy link
Member Author

@joergmschulz although the theming app ideally should be as self-explanatory as possible. :) Feedback on how it works is always good! For now we stick to the core 5 things:

  • name
  • website
  • slogan
  • color
  • logo

@LukasReschke LukasReschke added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jun 27, 2016
@LukasReschke
Copy link
Member

#59 (comment) all done

@jancborchardt
Copy link
Member Author

Should I also create new issues for the other 2 points in #59 (comment) or are they planned to be fixed as part of this? cc @LukasReschke @schiessle

@MariusBluem
Copy link
Member

at least the second point with the non-working button should be fixed here 😁

@MorrisJobke
Copy link
Member

Tested and works 👍

@LukasReschke
Copy link
Member

@jancborchardt check fe1089e and 4b79420. Testing appreciated.

@jancborchardt
Copy link
Member Author

jancborchardt commented Jun 27, 2016

Great stuff! 👍 works and looks nicely now

@LukasReschke LukasReschke merged commit a456291 into master Jun 27, 2016
@LukasReschke LukasReschke deleted the theming-app branch June 27, 2016 19:14
@jancborchardt
Copy link
Member Author

Yeeha, awesome work! :)

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants