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

Rss / Theme permission fixes #2826

Merged
merged 1 commit into from
Mar 27, 2016
Merged

Rss / Theme permission fixes #2826

merged 1 commit into from
Mar 27, 2016

Conversation

cmfcmf
Copy link
Contributor

@cmfcmf cmfcmf commented Mar 25, 2016

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Fixed tickets ---
Refs tickets #2819, #1776
License MIT
Changelog updated not yet

Description

This adds a new default permission allowing everyone to change to RSS, Atom and Printer themes. It also includes the theme name in the theme change permission check.

Todos

  • Tests
  • Documentation
  • Changelog
  • Test installation
  • Write upgrade procedure

@cmfcmf cmfcmf changed the title Rss / Theme permissions fixes Rss / Theme permission fixes Mar 25, 2016
@craigh
Copy link
Member

craigh commented Mar 25, 2016

please add schema definition in the composer.json file

@craigh
Copy link
Member

craigh commented Mar 25, 2016

I don't think this works for Core-2.0/ThemeEngine.

see https://github.com/zikula/core/blob/1.4/src/system/ThemeModule/Engine/Engine.php#L368-L399

@craigh
Copy link
Member

craigh commented Mar 25, 2016

I also don't think Core-2.0/Theme engine is checking the permission...

@craigh craigh added this to the 1.4.2 milestone Mar 25, 2016
@craigh
Copy link
Member

craigh commented Mar 26, 2016

@cmfcmf I finished this PR. please look it over and let me know if there are concerns.

Include selected theme in permission check.
@craigh craigh merged commit 108264b into 1.4 Mar 27, 2016
@craigh craigh deleted the theme-perm branch March 27, 2016 12:55
@craigh craigh mentioned this pull request Mar 27, 2016
4 tasks
@cmfcmf
Copy link
Contributor Author

cmfcmf commented Apr 2, 2016

@cmfcmf I finished this PR. please look it over and let me know if there are concerns.

Thank you for finishing!

@ghost
Copy link

ghost commented Apr 19, 2016

I upgraded to Zikula 1.4.2 and made the changes in News but I'm still getting the same error. Thoughts?

@rallek
Copy link
Contributor

rallek commented Apr 19, 2016

can you give me some more informations how you did the upgrade? Might be inside the ticket #2835?

@ghost
Copy link

ghost commented Apr 19, 2016

I did a clean install and then imported data from the old database to the new one. The installation went off without a hitch for me (thankfully) and I was able to move a lot of the existing content over. I didn't get any errors.

@ghost
Copy link

ghost commented Apr 19, 2016

The error is still

_System does not support the specified encoding.
Line: 1 Character: 38

_

I checked the URL at FeedValidator.org and got the following error:
_Sorry
This feed does not validate.
•line 14, column 4: Undefined feed element: item (25 occurrences) [help]

^
In addition, interoperability with the widest range of feed readers could be improved by implementing the following recommendations.
•line 1, column 31: Obscure XML character encoding: utf8 [help]

                           ^

•line 4, column 60: Relative href value on self link: / [help]

^
•line 4, column 60: Self reference doesn't match document location [help]

^
•line 5, column 11: title should not be blank [help]
<title></title>
^
•Non-RSS feeds should not be served with the "application/rss+xml" media type [help]_

@ghost
Copy link

ghost commented Apr 22, 2016

I had updated the index.tpl file in the News module and I also copied the index.tpl and view.tpl files to themes/RssTheme/templates/modules/News/user but I'm still getting the same error.

@rallek
Copy link
Contributor

rallek commented Apr 22, 2016

Did you also add the files in the ovewrite.yml?

@rallek
Copy link
Contributor

rallek commented Apr 22, 2016

@ghost
Copy link

ghost commented Apr 22, 2016

Would I copy that file to appropriate folder in the RSS Theme? Or would I create an overrides.yml file myself?

@rallek
Copy link
Contributor

rallek commented Apr 22, 2016

No idea. Give it a try to create one.

@ghost
Copy link

ghost commented Apr 22, 2016

I copied it from the Bootstrap Theme and changed the file paths to RssTheme with the same results.

@ghost
Copy link

ghost commented May 16, 2016

Has anyone else tried this to see if it works for them?

@ghost
Copy link

ghost commented Jun 10, 2016

It's still not working. I have made all of the changes in the various tickets and commits and I'm still getting the same error. It's quite frustrating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants