-
Notifications
You must be signed in to change notification settings - Fork 11
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
Upload files and ckeditor #31
base: development
Are you sure you want to change the base?
Conversation
…/redmine_gollum into ckeditor
I quickly reviewed your pull request (lot of changes!). It would be much easier to review if you split these into several pull requests: file upload, ckeditor, new page, raw view, list pages, list files. |
class CkeditorToGollumWikis < ActiveRecord::Migration | ||
def self.up | ||
add_column :gollum_wikis, :use_ckeditor, :boolean, :default => 0 | ||
add_column :gollum_wikis, :store_as_wiki, :boolean, :default => 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What this column is for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:store_as_wiki -- full name should be :edit_html_store_wiki, when you use ckeditor it edits HTML.
So if you want store files in wiki format AND use ckeditor it is necessary to convert wiki to html (easy) and after edit convert html to wiki.
If you don't care about store format and html is ok, then it is better to store ckedited documents in HTML.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see. This will only work for markdown. AFAIK, other formats don't support html.
As for a new page functionality: I see it in another way. It would be better to reuse and extend edit page: make it possible to name/rename pages on edit page. Then sidebar will contain a link to edit a page with empty name. Of course, there should be a warning if the page already exist... |
Upload file functionality: It would be better to have this form where it makes sense - on edit page. |
Raw view: would be glad to merge if it was a separate commit without ckeditor dependency. List pages and files: I would merge list pages, but without list files. Files are not necessarily images. |
It sounds reasonable. I'll think about it. :-) |
But there is problem: when you upload file in middle of editing you lost your changes. Solution could be set form target to _blank (or 'upload') to open response in new window. I solve it by adding sidebar to edit page, so you can open upload page in new window anyway. In ckeditor, there is tab to upload image, and it fill up image creation form automatically. |
Ok, but note that raw view have no point when you dont use ckeditor, you can simply edit page. (But yes, difference is, that possibly you have permission to read pages but not edit them). |
If ckeditor supports only html, I'm afraid I cannot merge these changes. Without conversion back and forth from html to markup, It poses a major vulnerability threat. And that conversion is a really ugly hack. The whole point of gollum and markup languages is that you can safely edit wiki with any text editor of your choice. Syntax highlighting and live preview would be great. But WYSIWYG is a weird & ugly concept, IMO. I'm thinking more towards integrating this livepreview implementation: https://github.com/bootstraponline/livepreview |
Ok. I respect it.
I don't get it. What is difference: if ckeditor can do conversion itself, it will be the same, but conversion will be on client side. Threats are the same or bigger (IMO).
I agree (but not uglier than most of wiki-to-html renders in Github::Markup). I wanted something functional as start point to future step-by-step improvements.
Yes, you correctly write 'IMO'. Other can have other opinion. That is why I make it optional. With 'use_ckeditor' set to no, and 'store_in_wiki' set to no, redmine_gullom is the same as ogrinal version. |
You're right. I missed the fact that gollum still sanitizes html.
Well, those additional columns is the major reason I don't want such a change. Since it's only the user's decision, making it a setting per wiki is kind of weird. Why not to create additional form parameters? Say, create a ckeditor tab with checkboxes 'store_as_inline_html/convert_to_markdown'. These can be saved in cookies. |
second service, now based on branch development.