Closed (fixed)
Project:
Drupal.org customizations
Version:
7.x-3.x-dev
Component:
User interface
Priority:
Normal
Category:
Feature request
Assigned:
Issue tags:
Reporter:
Created:
1 Oct 2015 at 22:22 UTC
Updated:
20 Jun 2016 at 17:34 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
drummThis will be www-specific. The other subsites can go at their own pace, and standardize among all the sites. (association.drupal.org has ckeditor, and is not configured up to the standards we'll want on Drupal.org.)
Comment #3
joshuamiMy first choice would be CKeditor with some HTMLpurifier configuration to make sure that people can only add just what they need to add.
Another option that would be pretty sweet would be to implement something like the Medium editor. That is such a clean editing experience.
Comment #4
markhalliwellWe've already adopted CKE in core and that will make it easier to upgrade *.d.o sites in the future.
As far as www-specific, I disagree. It is not good to have an inconsistent UI/UX across *.d.o sites.
If association.d.o isn't up to www.d.o's standards, then we should figure out what these standards are (or should be) and make it consistent across all *.d.o sites.
This issue should be more of a plan to discover requirements/standards with each site and then we can create new sub-issues to actually implement what each site will require.
I really do not think, nor believe, there should be much deviation amongst the sites.
Comment #5
tvn commentedComment #6
hass commentedHave you some how solved the issues that code and pre content gets not destroyes by conversions to html entities? The autocorrection always destroys html added inside a code or pre block for me. As i know this is a bug of ckeditor module or ckeditor itself, but I'm looking for years for a solution and cannot find any.
Comment #7
markhalliwellI really cannot comprehend why this issue was put in the "Drupal.org change notifications" email sent out on the Jan 21st given the lack of progress on this issue.
My comment in #4 has not been addressed and @hass makes a valid point regarding current d.o filters. These things need to be addressed. Furthermore, there needs to be ample testing regarding this implementation (e.g. a dev site) for at least a week, if not two (to give many people the opportunity), to ensure that things work as expect/desired.
I'm tagging so hopefully @nod_ can chime in and other JS savvy people. I also think it would be very wise for us to involve the core component maintainers for CKEditor: @Wim Leers (I doubt @mlewand will be needed?) with this issue as well.
If for some reason there has been discussions/decisions made elsewhere (e.g. IRC/hangout meetings that very few people attend) then the issue summary should have been updated or at the very least comments made reflecting this.
I feel like this is, yet another, "priority" that is being pushed through without regard to the community as a whole. It's quite clear that this issue needs more thought put behind it before "deploying" something that affects every single d.o member in the community (as has been the case far too often in the past with other "features/issues") and then cleaning up "mistakes" afterwards.
Transparency people.
Comment #8
japerryJust a note, we've implemented ckeditor on events and will be using some of those strategies on drupal.org. I hope to get a better synchronization of features with both of these properties as I work on ckeditor this week (and probably next).
Also, as part of my work in panels/layout/admin ui land -- I was pointed to this Editor (https://www.drupal.org/project/editor) as a potential better solution for drupal.org, especially as we get ready to move to Drupal 8 in the coming year (or two haha).
I don't foresee any major new features coming to WYSIWYG in the phase 1 deployment here. Basic replication of the current feature set is the primary goal. When I'm working on drupal.org, you can find those changes below. If you want a username/password, PM me on IRC and I'll get you added to my user-list.
http://japerry-drupal.dev.devdrupal.org/
Comment #9
japerryCurrently using these libraries on cod/events:
libraries[ckeditor][download][type] = "get"
libraries[ckeditor][download][url] = "http://download.cksource.com/CKEditor/CKEditor/CKEditor%204.3.4/ckeditor..."
libraries[ckeditor][type] = "libraries"
libraries[ckeditor_lineutils][download][type] = "get"
libraries[ckeditor_lineutils][download][url] = "http://download.ckeditor.com/lineutils/releases/lineutils_4.3.4.zip"
libraries[ckeditor_lineutils][type] = "libraries"
libraries[ckeditor_lineutils][subdir] = "ckeditor/plugins"
libraries[ckeditor_lineutils][directory_name] = "lineutils"
libraries[ckeditor_widget][download][type] = "get"
libraries[ckeditor_widget][download][url] = "http://download.ckeditor.com/widget/releases/widget_4.3.4.zip"
libraries[ckeditor_widget][type] = "libraries"
libraries[ckeditor_widget][subdir] = "ckeditor/plugins"
libraries[ckeditor_widget][directory_name] = "widget"
Comment #10
wim leers#2578693into our fancy issue links, with a preview of what it will actually look like. Just like Drupal 8 does for captioned images. Or to mention users. Or to refer to a prior comment.#8: pinged you to get a user/pass for that demo site.
#9: CKEditor 4.3 is outdated (and there have even been security updates in the mean time IIRC).
Comment #11
wim leersI forgot to reply to this:
+1.
Drupal 8 core ships with this by default since shortly before the release: #2549077: Allow the "Limit allowed HTML tags" filter to also restrict HTML attributes, and only allow a small whitelist of attributes by default — see the change record at https://www.drupal.org/node/2571349.
In Drupal 7, you would indeed want to install HTMLpurifier.
Comment #12
FredCK commentedMy one recommendation for this would be to avoid proposing the “source view". I mean, a way for writers to see the HTML and mess up with it at will.
I know that d.o. is developer oriented. Devs will always hate wysiwyg editor “limits”. The advantage of “no source view” is that it brings more focus on bringing a good editor solution and also (forcingly) teach writers to write right.
But ofc… it’s a case by case thing… not a must. My 2c.
Comment #13
mlewand commentedDefinitely sounds reasonable, I'd actually second to @wim-leers suggestion to start with as little plugins/features as possible and increase their number as you get more requests from content authors.
Avoid rushing it, make sure that everyone is comfortable with it.
Also think about CKE tweaks/enhancements to ease up your workflow. Sometimes you're able to create features that will save your authors a tons of time with a relatively low work/cost.
Comment #14
nod_If it's done well, that sounds like a good idea.
But I hope that we consider keeping a non-ckeditor fallback, for example I use opera mini (a proxy browser) since my phone can't handle much more, pretty sure ckeditor won't work there. But on desktop, sure sounds like a improvement.
Comment #15
wim leers#14: right, in that case, you'd stick with a no-JS, plain & simple
<textarea>. I think that's what you mean? If not, please clarify :)Comment #16
nod_That's correct.
Comment #17
japerryThere may be a further rabbit hole that using the D8 standard modules will put us in.
Specifically, if we go down the editor route, we will at a minimum will need to use:
https://www.drupal.org/project/dialog
Which brings us to the jQuery update module, and jQuery 1.7. Now on drupal.org (in the groups branch), I've been using jQuery 1.7 for a while. Maybe the lack of Javascript on drupal.org will make this a non-problem.
The could let us go down is the following additional modules later (which are all part of D8 core):
https://www.drupal.org/project/quickedit
https://www.drupal.org/project/entity_embed
-- And other modules that were implemented as part of the spark initiative.
Right now, after turning on the modules, I could get a basic site working with the editor, but the same configuration won't work on drupal.org. I just get 'loading...' and no errors (JS or PHP) and no dialog box.
So for now I think I'll continue with the plain ckeditor module path. We can investigate improving ckeditor with editor at a later date.
Comment #18
markhalliwellI would much rather go the "core"-ish route because it would make upgrading less painful in the future. The "do something quick and dirty now" method rarely serves us well and should, IMO, only ever be done for "small things". WYSIWYG is not a "small thing".
There's the @ mention issue which will also need to be taken into account. We'll have to have jQuery Update for that too.
It should be noted that upgrading d.o's jQuery version via jquery_update should actually be less painful using the 7.x-3.x branch since jQuery versions can now be independently set per theme. This should help alleviate completely or at least mitigate any JS issues we may run into.
Comment #19
japerryI think, in general, there is hesitation to implement jquery update on drupal.org. The amount of testing and random bugs it has caused on our other properties has meant that a very complete set of BDD tests would have to be finished and approved before we implemented jquery update.. This is on top of scanning the codebase and seeing if any functions fail when jQuery 1.7 is used. I want wysiwyg out sooner than later, and see this step as unnecessary to the MVP.
My tests of the editor module on drupal.org failed pretty spectacularly today... errors, bugs, etc. even though it worked fine on a smaller test site and client site.
I almost always agree with this statement. However, after initially leaning towards using editor module (which is the core-ish route), I'm now thinking ckeditor module will be better for the initial roll out. For two reasons:
Comment #20
markhalliwellI swear... no one actually fully reads my comments.
There's hesitation because, historically, jQuery Update has been an "all or nothing" type of deal. This is no longer true since we can now target jQuery per theme. Granted, this isn't 100% full proof since we'll have some module JS coming through, but generally speaking it should be fine. The majority of concern was around admin areas where things like Views UI would break (which we can leave alone with seven).
We have front end testing for Drupal??? Where?!
I would surmise the majority of "bugs" produced in jQuery "conflicts" is more or less a direct correlation to the lack of any solid coding standards for JS/jQuery prior to D8. It's also proportionate of certain contrib modules that "may or may not" fully understand how jQuery works, let alone JS itself. I seriously doubt we'd have much of those types of contrib modules on d.o.
Furthermore, I would suspect that the primary "hesitation" is because it has typically been BE devs doing FE work.... of course there's going to be hesitation. It's not really their area, it's ours. So we're, once again, sacrificing quality of code/site to meet an MVP doing what BE devs "thinks is best"??? When they run into FE bugs, who do they call? More BE devs.... go figure.
Here's a thought: How about they suck up their pride and ask us FE devs to help? You know... involve the community? I'm just a ping or tell away...
Switching anything in Drupal is not always as "easy" as it seems at first. You think doing the same on the FE will be any easier? Doing the "quick and dirty" isn't a solution. Period. It's a recipe for disaster. Especially considering the @ mention stuff I linked (which you're forgetting is also on the horizon too).
We'll need jQuery Update for that regardless.... so it's only a matter of time.
Time. The thing that is often the "excuse" for pushing through "features" at the expense of it's users. It's actually somewhat ironic. Dreditor used to be the "prototype" for d.o, but now d.o has been spitting out "feature" after "feature" like it's making up for all the time it lost trying to upgrade to D7 (which is really no longer the case btw).
So... one person doing one day of testing and it's a "no"? This is why I'm trying to bring attention (of other FE devs) to this issue. Just because the "email" said it was going to get "deployed" this week, doesn't mean it should happen. I still don't even know why it was in that email?! This is why we need to discuss and test this issue, over at least a couple of weeks IMO.
Comment #21
tvn commentedUpdating issue summary with some background on this feature request and more details on the scope of the implementation we are looking at in this particular issue. Our initial hope to deploy basic WYSIWYG editor by the end of this week was based on the plans to start working on this functionality at the beginning of the current sprint (in other words last Monday), however that didn't happen for a number of reasons. Our current focus is to have a prototype of CKEditor setup done on the dev site by the end of the week, so we could share it out for feedback and testing.
Comment #22
tvn commentedComment #23
tvn commentedComment #24
markhalliwellOh yeah... and there's this too.
Comment #25
japerryThe 'all or nothing' piece was fixed quite a while ago, (Edit: referring to admin vs end user) and like I said, we've been using it on our other properties. My personal hesitation is out of the real issues and compatibility problems prevalent when using a newer version of jquery. This isn't mere speculation, this is based from experience both recurring on our properties, and the initial attempt on drupal.org. jQuery Update is not plug and play.
I agree that a newer version of jquery, and the modules that can work with it, are something we should be planning for the future. But this is more than just ckeditor. This spans other javascript work we'd like to do to improve the frontend. Frankly, because this hits on so much of the site (almost the entire site uses bluecheese for instance -- and we'd want js features on the admin theme as well), it'd be irresponsible for us to throw it in as a dependency with other modules. It is out of scope for this particular issue and why I made the choice to throw out editor module as an option once I realized what else we'd have to deploy to support it.
So steering this back into ckeditor world, tvn has updated the issue summary, I've made a first run at the CKeditor experience, and added a url to test above. There are quite a few features still to work on here, but it'd be great to start testing. Thanks to those who've contacted me in IRC, you should all have login credentials. Let me know if you want to help out!
Comment #26
markhalliwellNot really, no. The "admin specific" attempt in 7.x-2.x was inherently flawed due to how Drupal determines it's "admin paths". The convolution of when/where jQuery Update would replace it did not always actually correlated to the correct theme when on an admin path (which is almost ~90% where stuff gets foo-barred). Which is why I helped and committed the "theme" solution to 7.x-3.x (co-maintainer here btw). Bluecheese it self has little to no JS. And what JS is used on the FE aspects of d.o is provided by very few modules, most of which likely won't fail.
Correct. It's not. That's why FE people like me have jobs that specialize in the FE and FE architecture. This is my job. I know what I'm talking about. You're ignoring this fact and giving up by saying "it doesn't work" when, in fact, it would if you asked for our help to fix things.
It shouldn't be. Which is the whole point y'all are missing.
I get that y'all want this to be a "minimal implementation:", but this is a highly unrealistic "goal" given the current state of d.o's JS quality (or lack thereof) and especially when everything (I have already mentioned) is coming down the pipeline.
All this JS will have to address at some point anyway. You cannot just "throw in" CKEditor and then "expect" a clean 1:1 upgrade in the future when all evidence to the contrary supports that *.d.o generally encounters some sort of [relatively major] issue(s) during an upgrade (despite best efforts/intentions).
It's clear that there is obviously NO concern that there will not be any future consequences and y'all are thinking that adding more standalone module/JS will help things?! Here's a thought.... give a thought! Plan some architecture around this instead of just trying to "get something out there"... this approach/attitude has NEVER served d.o and/or it's users well in the past.
It's like y'all are completely ignoring all of these FACTS. But sure... do what y'all want, y'all always do....
Comment #27
joelpittet@japerry I don't mind helping get
jquery_updateworking on drupal.org, if it's not a requirement to make ckeditor work that's fine but I've also ran through the gauntlet as have many and the pitfalls are few depending on which version you decide to deploy. 1.7 is very safe for admin and front-end (a little less on admin for views/rules) though most of those issues have been or can be resolved if needed.Though if it's not required to get it working, no need to add unnessasary dependencies to resolve getting ckeditor on d.o.
Also +1 to using ckeditor module.
Comment #28
japerryAfter being deflected the last few weeks, I've had a chance to come back to ckeditor.
This feature will be getting deployed with the prism work being done here: #2629046: Add syntax highlighting to Drupal.org
If you want to test the latest features, please go here: https://drupal:drupal@japerry-drupal.dev.devdrupal.org
Some things to test:
Comment #29
japerryComment #30
wim leersI never got a working login to test this. Can you send me one?
Comment #31
hestenet@Wim - Try bacon/bacon and an issue like this one: https://japerry-drupal.dev.devdrupal.org/node/2646434#comment-10823194 to see an example
Comment #32
nod_Using opera mini (I couldn't get UC browser to work because of htaccess) the CKEditor interface sort of loads but is inactive. It's impossible to post a comment. CKEditor is also configured to be used in the forums so it's a pretty big deal. We have many people from India and there UC browser has a very big market share. We should at least make sure it works on that browser.
Comment #33
japerrynod_, Do you know if Opera Mini and UC Browser work with Drupal 8's ckeditor implementation?
While I'd like to have those browsers work, they represent such a small percentage of our traffic (less than half a percent) that I don't know if its worth dedicating time on it.
Comment #34
nod_I tested UC Browser on D8 and it works as expected. It's only an opera mini issue at this point, which is sort of good news.
The implementation on events.drupal.org is great tho. When I browse with my puny browser I get a textarea and when I'm on desktop I get ckeditor + switch to plain text link. Any change we can reuse that ? I'd really like a switch to plain text link, just in case. Since ckeditor is huge and take a long time to init on mobile, having the option to disable ckeditor by default would be nice. loading 100+ comments is already a pain, no need to add a couple of seconds for ckeditor.
Comment #35
markhalliwellFor the record:
I still believe this is the completely wrong approach, but it would appear "progress" is continuing despite my warnings.
---
Also, this issue is doing a hell of a lot more than just "minimal implementation" as the IS states that should happen.
Templates are an entirely separate issue, one that should allow per project configuration.
Dreditor is also "breaking" left and right. Granted some of it is completely unavoidable given that CKE is essentially incompatible with Dreditor.
Thus, it would seem that I have no choice in this matter and forced to start "fixing" Dreditor in preparation for this major change.
---
I did take a look at the link above and everything looks "cool" and seems to work just fine.
I'm not going to stand in the way of this (as if that is really possible anyway), but just know that I'm still extremely annoyed with how the DA is handling issues like this.
Comment #36
markhalliwellAlso, @gisle brought up a valid point on #2191525: [PP-1][policy, no patch] Extend Markdown coding standards to support API module (DOXYGEN): g.d.o currently has the markdown filter enabled. How will CKE handle this exactly considering that the primary purpose of CKE is to visually represent HTML and not markdown?
Comment #37
japerryIn general, gdo is being upgraded this summer. Work being done here will not flow to gdo. When content from gdo is migrated to d.o, we'll tackle that then. Most likely it'll be converted to html (much like we're converting php and code), which will eliminate the markdown code.
As mentioned in the linked issue, we hope to give better functionality for templates. However, the current functionality is what we're looking to deploy as a first run. Once that is done, we can circle back to our prioritization process and work more on per-project templates.
Thank you for working on making dreditor compatible.. yes this will be a fairly major change.
Comment #38
hestenetComment #39
japerrySorry! I should have marked this postponed last week -- but just a short update, we're deploying and testing Prism (Syntax highlighting) before we work again on ckeditor.
Comment #40
wim leersRegarding dreditor: breaking that almost seems like a non-option. Lots of developers heavily rely on it. OTOH, dreditor could just destroy all CKEditor instances for now, that would literally be a few LoC.
Comment #41
wim leersI tried logging in, but I'm still not able to.
I'd love to review code instead, but I can't find where a patch or branch is for this?
Comment #42
hestenet@wimleers - There was a data loss issue on our dev sites earlier this week. We'll be rebuilding this dev site so more folks can look. Sorry for the inconvenience.
Comment #43
wim leersnp, thanks for letting me know it's not me doing something wrong :)
Comment #44
japerryNow that prism is deployed, ckeditor work has started back up. I've restored and updated the devsite. you can use the user: bacon/bacon to browse the site as a regular user.
So far I have initial templates in, and the codesnippet. Still need to fix problems with prism rendering and the wrong code being saved with existing code and php tags.
https://japerry-drupal.dev.devdrupal.org/
Comment #45
markhalliwellFWIW, the issue summary template code in Dreditor has been removed (from 2.x branch) in preparation for this issue.
See: https://github.com/unicorn-fail/dreditor/issues/262
Edit: there is still a ton of work that needs to be done to Dreditor though
Comment #46
japerryThanks Mark for the update.
Here is our first pass through on the devsite. Looks like there are a few dreditor issues remaining, but we haven't done testing with the 2.x branch yet.
https://docs.google.com/spreadsheets/d/1n2t1n44RsCWeZrKps-h3hQquXScm7JmG...
Comment #47
wim leersRepeating what I said in #41: I'd love to review code, but can't, and I'd love to give it a try (per #44), but I can't.
Comment #48
japerryYour password should be set to the default.
The branch with the ckeditor changes (drupalorg_wysiwyg feature) is in here:
http://cgit.drupalcode.org/drupalorg?h=7.x-3.x-groups
Bluecheese changes are in our private repo, but the patch (which includes the templates) is here.
Comment #49
japerryHere is what we're getting ready to deploy:
https://ckeditor-drupal.dev.devdrupal.org
A few remaining items:
Otherwise, I think this is pretty close.
Comment #50
hestenetDA staff are reviewing https://ckeditor-drupal.dev.devdrupal.org for potential deployment on Monday. We've done several rounds of browser testing and it's behaving well in FF, Chrome, Safari, and tolerably in Edge, IE11 (with some bugs that are upstream to CK moreso than part of our implementation).
I also tested against the 2.x branch of dreditor - and we do have some concerns there - the big one being: https://github.com/unicorn-fail/dreditor/issues/283
Basically - Dreditor loses the ability to focus on the text area when pasting in code review or image embed. It still works in plain text view, but that's not ideal. Would appreciate some help from the Dreditor folks or especially javascript savvy people on the dreditor github issue above to try and reduce disruption for heavy Dreditor users with this change.
Comment #51
hestenetComment #52
drummAdding some related issues for this deployment. In general, https://bitbucket.org/drupalorg-infrastructure/drupal.org/commits/branch... is what we’re getting ready to deploy. I’ll be helping review the individual pieces, and putting them in production as they are ready.
Comment #53
hestenetComment #54
hestenetAfter more internal review, we've decided to deploy CKEditor to a limited set of content types, to allow a larger audience to review before we push to issue queues.
Comment #58
drummTo get a good comparison with production, I've exported our existing input formats to compare with the branch’s. There are 3 differences:
<s>tag is now allowed. I went ahead and put this in production.<address>tag is added in the branch. I’ll probably be removing that from the branch.Comment #60
drumm<address>has been removed from both the allowed tags and CKEditor configuration.#2136887: Process code filter before line breaking does look like a good change. It doesn't help #2725111: < within <?php is not escaped for codefilter_prism, and doesn’t hurt it either. I’m planning on making this change in production Thursday AM, to give us a day to notice regressions.
Comment #63
hass commentedStill wondering how you solved #6.
Comment #64
drumm#2704949: When using prism, the pre wrappers get infinitely wrapped on ckeditor is the key change for not over-encoding code. Our setup is a bit intertwined, the whole Drush make file is in https://bitbucket.org/drupalorg-infrastructure/drupal.org/src. Until deployment, ckeditor's branch is
7.x-integration.Comment #65
drummI'll be deploying #2704949: When using prism, the pre wrappers get infinitely wrapped on ckeditor on Drupal.org shortly.
For CKEditor itself, initially we will limit it to the
'page', 'post', 'section', 'guide', 'documentation'content types, which are all admin-only at the moment. This is generally looking okay for launch next week. Additional content types, like issues, will be in followup issues.Comment #68
drummI’ve gotten all the prerequisites that I want out of the way and testing looks okay. The straggling issues were from having two configurations for the two input formats that like to diverge. Deployment for the initial set of content types can be done as early as later today.
Comment #69
wim leersI still haven't had a chance to test/review the CKEditor integration on a staging site. I'd like to be able to do so — as the maintainer of the CKEditor module (and hence integration) in Drupal 8, I likely have useful feedback that will avoid problems down the line.
Comment #70
hestenet@Wim - https://drumm-ck2-drupal.dev.devdrupal.org/node/2603760/edit
htauth: drupal/drupal
user/pass: bacon/bacon
I've elevated the privileges of this user so you can test on the content types we're deploying to first: Page, Post, and Section, and then the new Documentation Guide and Documentation Pages.
Comment #71
drummI filed #2578695: Use CKEditor on *.drupal.org sites with some followups I’ve noticed. This first deployment will of course not be perfect, since it is limited by content type, it will help start thorough testing.
Comment #72
drumm#2741227: Enable CKEditor for more content types is the followup issue.
Comment #73
drummThis initial deployment is done, let's collect followup improvements at #2741227: Enable CKEditor for more content types.