Problem/Motivation

While working on #1678002: Edit should provide a usable entity-level toolbar for saving fields, it became more apparent that our Create.js/VIE.js architecture in the Edit module would not support (easily) the interaction pattern we're trying to build. Namely that one interactions primarily with an entity for in-place editing -- one edits fields in the context of an entity and saves changes in the context of that entity.

jessebeach took several days to determine if it would be feasible to factor Create/VIE out of the Drupal codebase #1678002-71: Edit should provide a usable entity-level toolbar for saving fields and we determined yes, we could do it in a reasonable amount of time and effort. Wim Leers developed these reasons for the refactor in #1678002-72: Edit should provide a usable entity-level toolbar for saving fields:

  1. It depends on VIE.js, which does five zillion things we don't need; it assumes RDFa (which is "not correct/accurate enough" in Drupal, see earlier issues) and JSON-LD (which won't be in D8 core after all). It's *huge*. 34 KB minified! We're currently only really using a tiny, tiny subset of VIE.
  2. Create.js deeply depends on jQuery UI Widgets, which are a huge PITA to use. It's extremely confusing. We've been wanting to refactor that out of Create.js, but that is a *huge* undertaking. And it's not clear how we'd retain backwards compatibility, which is a requirement.
  3. https://github.com/bergie/create/issues/133, https://github.com/bergie/create/issues/137 -> Create.js doesn't have per-field state change handling, like we need. Instead, we've been pretending for all the time that Create.js has been in core that each field is an entity, which is obviously not the case. Now that we need entity-level state AND field-level state, that breaks down. It's possible to hack around, but… it requires a significant change in Create.js and it will break backwards compatibility. Bringing those changes to Create.js (in a BC way, mind you!) is likely going to be at least equally complex as moving away from Create.js. And that's not the only issue. And we'd still be fighting jQuery UI Widgets. Etc.
  4. Overall, Create.js makes *way* too many assumptions. Because it was originally built as a specific implementation of in-place editing, with its own UI. Whereas it now claims to be a platform for in-place editing. That's not really true yet.
  5. It's abstraction for in-place editors ("PropertyEditor widgets") is not really an abstraction. It, too, makes way too many assumptions. It does event binding+handling, which it should not do *at all*. That's why we've been unable to reuse or even just subclass Create.js' PropertyEditor widgets.
  6. https://github.com/bergie/create/issues/142
  7. With all of the above, the removal of Create.js and particularly the removal of VIE.js would result in a huge decrease of the amount of JS to be downloaded, parsed and executed for in-place editing.

The most salient reason from the list above is this one:

Create.js doesn't have per-field state change handling, like we need. Instead, we've been pretending for all the time that Create.js has been in core that each field is an entity, which is obviously not the case. Now that we need entity-level state AND field-level state, that breaks down. It's possible to hack around, but… it requires a significant change in Create.js and it will break backwards compatibility.

We will keep #1678002: Edit should provide a usable entity-level toolbar for saving fields focused on the introduction of an entity-level toolbar for in-place editing. This issue here will become the home of the Create/VIE out-factoring.

Proposed resolution

Remove Create/VIE from Drupal. Refactor the existing Edit JavaScript to leverage Backbone Models and Views. This is already the case to some degree, but because Create uses jQuery UI to handle Widget creation and eventing, these aspects of the application will need to be ported to Backbone. Most of this porting is already completed and working as before.

Remaining tasks

Separate #1678002: Edit should provide a usable entity-level toolbar for saving fields into two issues and move the refactor code here.
Finish porting functionality to the new Backbone Models and Views.

User interface changes

See #1678002: Edit should provide a usable entity-level toolbar for saving fields.

API changes

Some, but since no code is leveraging these systems yet outside of Edit/Editor, the changes will not break anything.

#1678002: Edit should provide a usable entity-level toolbar for saving fields

CommentFileSizeAuthor
#36 edit-create-refactor-1979784-36.patch390.4 KBWim Leers
#36 interdiff.txt852 bytesWim Leers
#30 edit-create-refactor-1979784-30.patch392.78 KBjessebeach
#30 interdiff_28-to-30.txt18.51 KBjessebeach
#29 edit-create-refactor-1979784-29.patch383.89 KBWim Leers
#29 interdiff.txt681 bytesWim Leers
#28 edit-create-refactor-1979784-28.patch383.89 KBWim Leers
#28 interdiff.txt8.6 KBWim Leers
#27 edit-create-refactor-1979784-27.patch382.64 KBWim Leers
#27 interdiff.txt72.91 KBWim Leers
#26 edit-create-refactor-1979784-26.patch376.92 KBWim Leers
#26 interdiff.txt59.99 KBWim Leers
#22 edit-create-refactor-1979784-22.patch378.38 KBWim Leers
#21 edit-create-refactor-1979784-21.patch377.98 KBWim Leers
#21 interdiff.txt9.2 KBWim Leers
#18 edit-create-refactor-1979784-17.patch377.42 KBjessebeach
#17 edit-create-refactor-1979784-17-do-not-test.patch377.42 KBjessebeach
#17 interdiff_16-to-17.txt2.6 KBjessebeach
#16 edit-create-refactor-1979784-16-do-not-test.patch374.51 KBWim Leers
#16 interdiff.txt24.9 KBWim Leers
#15 edit-create-refactor-1979784-15-do-not-test.patch368.55 KBWim Leers
#15 interdiff.txt634 bytesWim Leers
#14 edit-create-refactor-1979784-14-do-not-test.patch368.55 KBWim Leers
#14 interdiff.txt42.38 KBWim Leers
#12 edit-create-refactor-1979784-12-do-not-test.patch361.62 KBjessebeach
#10 edit-create-refactor-1979784-10-do-not-test.patch360.03 KBjessebeach
#10 interdiff_9-to-10.txt5.64 KBjessebeach
#9 edit-create-refactor-1979784-9-do-not-test.patch356.46 KBjessebeach
#6 edit-create-refactor-1979784-6-do-not-test.patch358.68 KBjessebeach
#6 interdiff_3-to-6.txt206.72 KBjessebeach
#3 edit-create-refactor-1979784-3-do-not-test.patch203.19 KBWim Leers
#3 interdiff.txt54.13 KBWim Leers
#2 edit-create-refactor-1979784-2-do-not-test.patch211.16 KBjessebeach
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

webchick’s picture

Issue tags: +sprint, +Spark

Taggity tags.

jessebeach’s picture

Status: Active » Needs work
FileSize
211.16 KB

Here is the port from #1678002: Edit should provide a usable entity-level toolbar for saving fields with just the refactor code and none of the entity toolbar changes.

Wim Leers’s picture

Component: javascript » edit.module
Assigned: Unassigned » Wim Leers
FileSize
54.13 KB
203.19 KB

Changes:

  1. Saving is working for the 'form' in-place editor!
  2. Reverting of changes is working! (It is broken in D8 HEAD.)
  3. EditorView is back, was removed in #2. Now provides a base class for other in-place editors, with docs and some default method implementations.
  4. Moved a bunch of things from edit.js to AppView.
  5. #2 removed EntityView, but didn't remove the instantiations.
  6. Fixed a regression in findEditableProperties.
  7. Fixed lack of Editor/Toolbar/Decoration views when a modified field has been saved.
  8. storage.js is gone.

Next: make the 'direct' editor work again.

henribergius’s picture

All of the issues mentioned are definitely fixable, though I would've preferred communication along the lines "here are the things we need you to fix for us to be able to keep using it" than the way it is said now.

I still believe in benefit of using a common codebase shared between CMSs instead of everybody trying to reinvent (and maintain!) all the wheels themselves. For a longer version, please refer to http://bergie.iki.fi/blog/drupal-and-collaboration/

The question is, then: how set are you with doing all that by yourselves? Certainly the changes we made in the process of integrating Create.js -- like porting your Edit module to Backbone -- make this a lot easier. But maintaining the full editing stack is a lot of work. Trust me, I should know ;-)

Now, going to the concrete issues:

1. VIE is indeed a big library, especially if you build the services in. However, it is in no way locked into RDFa (or JSON-LD), but merely uses those as defaults. As I've said to Wim, there are several places and ways in which we can make VIE smaller if there is a need

2. jQuery UI widgets are a handy way of connecting stateful JavaScript code with DOM elements. The breakage caused by 1.10 was a bummer, of course. We already had plans to migrate the core functionality of Create to regular Backbone views and models, but to keep the jQuery widgets around as API compatibility wrappers

3. Create has per-field state handling, but with the assumption that the editors would drive state changes. In Edit, the idea is to move this around, which causes some complications. Again, fixable with a bit of time

4. Initially Create was, indeed, built with its own UI (and we still ship that as a "sane default"). But as the integrations with Drupal's Edit (Backbone), OpenCms (GWT), and TYPO3 Neos (Ember) have shown, it plays also quite nicely with a custom UI. The main assumption causing problems for Edit was already covered by 3.

5. This would need a bit more discussion

7. Sure, there would be at least some wins in download size. But at the cost of having more code "proprietary" to Drupal, and so maintained only by your community

Anyway, we can discuss this further when you have the report of your reimplementation done. I would've liked to hear about this sooner so individual issues could've been addressed one by one. But hey, I can't have everything!

Wim Leers’s picture

We'd like to expand on the context of how we reached the undertaking of this experiment. Last week (around 4/17/2013), jessebeach was trying to introduce an entity toolbar element to the Edit module (#1678002: Edit should provide a usable entity-level toolbar for saving fields). After a couple days of halting attempts to work this entity-layer of abstraction in the Create/VIE framework and our Edit JS on top of that, she decided to see what Edit might look like without Create/VIE. This was no simple matter and it took the entirety of a weekend to get a quick-and-dirty proof of concept in place that suggested it might be possible. The prototype also seemed to support an entity-level toolbar in a way that wasn't easily possible with the previous codebase.

Spurred by the initial results, we continued this week working this proof of concept into a patch (#1979784: Factor Create.js and VIE.js out of the Edit module — this issue), which we posted yesterday. This has been a fast process. We did not know what the results would be. We wanted to see what the code would look like. Our intent was to reach out to you, Henri, once we had our heads wrapped around what exactly we're proposing. We did not intend for you to be informed of this work through Twitter; rather, once we had code to compare (i.e. facts to compare instead of statements like "the code would be so much better if X and Y"), we wanted to reach out and have a discussion with you, which we planned to do today since this patch was posted late in the evening European time.

Given the design direction of the entity toolbar and the realities of Drupal's entity/field modeling, it looks more and more like we have to take a direction that moves us away from Create and VIE. We are truly sincere when we say that we always prefer sharing code rather than rolling our own. But requirements, performance, timelines and supporting the Drupal contrib space pose us a tough decision. We think the benefits of the approach we're proposing in this patch are substantial at this moment.

We reached out via e-mail to have a call about this ASAP.

jessebeach’s picture

This patch rebases on HEAD (9c996a8f78dd29df0270e5af15b4e8d172ebcc45).

Changes include:

  1. Removed VIE library files.
  2. Removed all references to Create.js and VIE (code and comments) in Edit and Editor.
  3. Renamed ToolbarView to FieldToolbarView and PropertyEditorDecorationView to EditorDecorationView

stats: 46 files changed, 2305 insertions(+), 7674 deletions(-)

henribergius’s picture

Commenting on this, and the email report of your effort that you received.

Apart from Create.js itself, I believe a major contribution we did to the Edit codebase was porting it to Backbone. This gives me confidence that you'll be able to produce a maintainable inline editing solution, with or without Create.

However, by bailing out of Create, you're making a bet against the shared CMS JavaScript ecosystem. If merely integrating Create improved your own JS codebase, what would be possible once the multiple Create and VIE based editing UIs are actually out there and use? By withdrawing, you'll be missing out on whatever the world outside of Drupal comes up with.

As for the more concrete issues you've faced. I agree that the concept of Editables as integrated now doesn't fit Drupals "edit each field separately" paradigm. In Create the assumption is that people are editing and publishing entities as a whole, not just fields.

But this would be easy to work around by treating each Drupal field as a separate entity. This way you'd have all the control and state you crave with the existing features that Create can offer. And you could even use features like collection editing for dealing with multivalued fields (if Drupal has support for those).

That said, I see you're more or less determined to push ahead. I'm sad to see you go, as Drupal would've been a valuable participant in the decoupled CMS world. But can't win them all...

Wim Leers’s picture

@henribergius, thank you for the considered response. We are fairly confident that the approach we're taking — removing the Create.js and VIE dependencies — is what we need to do at this moment in time given our current schedule and design constraints.

As a rule we always want to work with community projects that align with Drupal's needs. Even though that alignment shifted for us now because of RDFa & JSON-LD support and our entity-based architecture, that doesn't mean that we might not come back into alignment later. We might find ourselves revisiting this issue as we spin up Drupal 9. Jesse and I and the rest of the Drupal front end folks will take any and all options on the table and consider their benefits.

jessebeach’s picture

reroll on (9ae545f7544dd7b66d6b2f9e95fc35277d8ed44a).

jessebeach’s picture

This patch reintroduces the save flow for direct editables. At least, that's all I've tested it with. It "might" work with form editables.

The validation code still needs to be cleaned up.

Wim Leers’s picture

Making Edit leverage Drupal.TabbingManager (#1844220-15: Make in-place editing keyboard and aurally accessible) is blocked on this issue.

jessebeach’s picture

Rebased on 8.x (50ae3296382669baebee8b9afa909361eb5aa412).

Wim Leers’s picture

Working on a big step forward, please don't reroll in the mean time.

Wim Leers’s picture

  • Removed edit.css whitespace changes. (That was all that was changed there.)
  • Defined Drupal.edit.EntityCollection, like Drupal.edit.FieldCollection — to remove one-off code.
  • The rebase in #12 was completely wrong, it ignored upstream changes introduced by . E.g. ContextualLinkView.js removed all changes in #1981760: Edit should use the new drupalContextualLinkAdded event to cleanly instead of hackily insert its "Quick edit" contextual link. Worse, all code in edit.js has been merged completely incorrectly, leading to duplicate, erroneous code. Fixed all that. It was only working by coincidence… :)
  • Moved methods out of Drupal.behaviors.edit, because nod_ doesn't like that — he prefers us to use functions that are only available within the closure. He thinks only (de)(at)tach methods should exist on Drupal.behaviors[modulename]. Complying with that.
  • We have asynchronously retrieved field metadata. Contextual links may be added asynchronously. And we want to ensure that our "Quick edit" contextual link action gets set up only when it makes sense: when there are >=1 in-place editable fields for the entity on which the contextual links exists. And only *then* should FieldModels be created, because otherwise they're just consuming memory and not doing anything useful.
    So we really need three asynchronously updated queues: 1) fields without metadata, 2) fields that have been verified to be in-place editable by the current user, 3) contextual links which qualify for a "Quick edit" link
  • The three above points eventually equated to an almost-rewrite of edit.js. At the same time, that rids us of organically grown complexity that needed to be refactored anyway, and of the last remnants of "updating the DOM to allow VIE to work cleanly". On the downside, our previous refactoring steps got rid of the ability to destroy things properly when a field got removed. That's now also fixed.
    There are now just a handful of functions in edit.js' closure, that I tried to make as clear and concise as possible: initEdit, processField, initializeField, fetchMissingMetadata, initializeEntityContextualLink and deleteContainedModelsAndQueues. Everything is well-documented now.

I had expected to work on more directly useful things, but this really needed to be done. The changes made in this interdiff should allow faster progress now though :)

Wim Leers’s picture

The patch in #14 contains one stupid small mistake.

Wim Leers’s picture

In this reroll

So, back to what I really wanted to work on. I said in #3:

Next: make the 'direct' editor work again.

Changes

  • backbone.drupalform.js is no more. It doesn't make for us to use Backbone.sync, because we do not have a REST API that allows us to create content *and* validate that content. It certainly does not make sense for the 'form' in-place editor, which is the one used for most fields currently (i.e. all non-textual fields, all structured data fields).
  • The VIE-era EditService.js contained work-arounds to distinguish between fields that would be "directly" in-place edited (i.e. the value itself would be in-place edited, which almost certainly equates to textual editing) and those that would not be. After all, a single field may contain many items (cardinality >1) or a single item (cardinality == 1). So, in short: before this patch it was very hacky, and all above patches did not yet deal with this.
    The new approach to deal with this is very simple: a new, required EditorView.getEditedElement() method:
        /**
         * Returns the edited element.
         *
         * For some single cardinality fields, it may be necessary or useful to
         * not in-place edit (and hence decorate) the DOM element with the
         * data-edit-id attribute (which is the field's wrapper), but a specific
         * element within the field's wrapper.
         * e.g. using a WYSIWYG editor on a body field should happen on the DOM
         * element containing the text itself, not on the field wrapper.
         *
         * For example, @see Drupal.edit.editors.direct.
         *
         * @return jQuery
         *   A jQuery-wrapped DOM element.
         */
        getEditedElement: function () {
    

    The 'direct' and 'editor' in-place editors can now just say "hey, we don't care about editing this field wrapper thing, we care about the first and only field item within the field wrapper".

  • Thanks to the above, the 'editor' in-place editor is now working perfectly again. (It already was working correctly thanks to CKEditor magic, but there was a visual flaw.) It can now also again load the untransformed variant of a piece of transformed text.
  • The 'direct' in-place editor is now also working perfectly.
  • The leaky abstraction that we had before, where FieldToolbarView had to go and clean up the remnants of the hidden form that's being used to save data. That's now all taken care of by EditorView.save().
jessebeach’s picture

Status: Needs work » Needs review
FileSize
2.6 KB
377.42 KB

backbone.drupalform.js is no more.

Awesome! This always felt like a hack, albeit a necessary one. The save method on EditorView is much more grokkable. In fact EditorView really exposes plainly the editing interaction model now. Before, it had been half in Create, half in Edit and it was unclear which code was responsible for what.

Awesome, awesome work Wim!

We're finally boiling the code down to just what's needed and little more.

I added overridden remove methods to the EditorDecorationView and the FieldToolbarView. We'd only been calling undelegateEvents on the views, which is a temporary way to remove the views. It doesn't stop the view from listening to model change events. By calling the Backbone.View.prototype.remove method on the views, we remove it correctly.

We're ready to start reviewing this code.

Wim suggested we might move the deletion of Create.js and VIE to another issue so that the diff here in this patch isn't so large.

jessebeach’s picture

Oops, I wanted the bot to run!

Status: Needs review » Needs work

The last submitted patch, edit-create-refactor-1979784-17.patch, failed testing.

Wim Leers’s picture

I created a minor follow-up task where we can document which name changes we want to introduce that are not necessary (i.e. don't belong) for this refactoring: #1989974: [meta] Clean up/improve consistency of Edit.

Wim Leers’s picture

Status: Needs work » Needs review
FileSize
9.2 KB
377.98 KB
  • Removed a bunch of dead code.
  • Moved the basic settings out of the edit library and into the page-level attaching. That way, the same settings don't get attached on every metadata callback. Much better.
  • Got tests passing again.

Next up: to a reasonable degree, simplify things and make them more consistent. Particularly FieldModel. Do some renaming too. Since Jesse's in agreement with the patch in #16, we can now do that last bit of work as we are approaching RTBC :)

Wim Leers’s picture

Oops, I excluded #17. Fixed that. Interdiff at #21 is still valid.

Status: Needs review » Needs work
Issue tags: -JavaScript, -sprint, -Spark, -edit-followup

The last submitted patch, edit-create-refactor-1979784-22.patch, failed testing.

Wim Leers’s picture

Status: Needs work » Needs review
Issue tags: +JavaScript, +sprint, +Spark, +edit-followup
Wim Leers’s picture

Big interdiff, but this is just changing indentation and removing jQuery.extend where it is not needed.

Before:

$.extend(Drupal.edit, {
  AppView: Backbone.View.extend({
    stuff();
  })
});

After:

Drupal.edit.AppView = Backbone.View.extend({
  stuff();
});
Wim Leers’s picture

This patch should be RTBC'able, or close to RTBC'able. Please review! :)


Changes

  • Renamed editID in all JS code to fieldID. It is now also FieldModel's id attribute, instead of editID in there. So: field ID (formerly edit ID) at fieldModel.id, entity ID at entityModel.id. Nice & simple.
  • Simplified FieldModel as far as possible!
  • Split out all the in-place editor state away from FieldModel, into a new EditorModel. As a side-effect, EditorModels only exist for the duration that EditorViews exist — which makes a lot more sense. (Previously, a FieldModel would continue to remember the last state of the last in-place editor, even after quick editing of an entity was stopped.)
  • Renamed AppView.(un)decorate to AppView.(setup|teardown)Editor.
  • Everything (well, >98%) is now properly documented.
  • edit_contextual_links_view_alter() -> edit_page_build(). More logical given recent contextual links changes. Copied from #914382: Contextual links incompatible with render cache.
  • General code clean-up & simplification.
  • More dead code removal.
  • s/function(/function (/, because nod_ would definitely complain about that otherwise :)

Also note the clean-up & consistency improvements list that got updated at #1989974: [meta] Clean up/improve consistency of Edit. We don't want to do those things here because those changes are unrelated to the task at hand: factor Create.js & VIE.js out of Edit module's JS.


When testing, try doing something like Drupal.detachBehaviors(jQuery('.block-custom-block')), you'll notice that Drupal.edit.collections.(fields|entities) are updated, and in fact that the "Quick edit" link no longer appears in the custom blocks' contextual links! This never worked before.


Before

wim.leers at wimleers-acquia in ~/Work/drupal on 8.x*
$ grunt
Running "concat:edit" (concat) task
File "build/edit.js" created.

Running "min:edit" (min) task
File "build/edit.min.js" created.
Uncompressed size: 76329 bytes.
Compressed size: 7686 bytes gzipped (27835 bytes minified).

Running "min:createjs" (min) task
File "build/create.min.js" created.
Uncompressed size: 51818 bytes.
Compressed size: 5712 bytes gzipped (23643 bytes minified).

Running "min:viejs" (min) task
File "build/vie.min.js" created.
Uncompressed size: 133810 bytes.
Compressed size: 9350 bytes gzipped (32370 bytes minified).

Running "min:backbone" (min) task
File "build/backbone.min.js" created.
Uncompressed size: 94306 bytes.
Compressed size: 9905 bytes gzipped (30252 bytes minified).

Running "min:jquiwidget" (min) task
File "build/jquiwidget.min.js" created.
Uncompressed size: 23265 bytes.
Compressed size: 3968 bytes gzipped (10560 bytes minified).

Running "min:all" (min) task
File "build/all.min.js" created.
Uncompressed size: 379532 bytes.
Compressed size: 34736 bytes gzipped (124652 bytes minified).

Done, without errors.

After

wim.leers at wimleers-acquia in ~/Work/drupal-tres on 8.x*
$ grunt
Running "concat:edit" (concat) task
File "build/edit.js" created.

Running "min:edit" (min) task
File "build/edit.min.js" created.
Uncompressed size: 84320 bytes.
Compressed size: 7437 bytes gzipped (27923 bytes minified).

Running "min:backbone" (min) task
File "build/backbone.min.js" created.
Uncompressed size: 94306 bytes.
Compressed size: 9905 bytes gzipped (30252 bytes minified).

Running "min:all" (min) task
File "build/all.min.js" created.
Uncompressed size: 178627 bytes.
Compressed size: 16999 bytes gzipped (58173 bytes minified).

Done, without errors.
Wim Leers’s picture

Fixed initial feedback from nod_:

  • EditorModel not in patch. Oops!
  • VIE.js lib declaration is still in system.module
  • Drupal.edit.Metadata should be Drupal.edit.metadata because it's not a constructor
  • Using jQuery when the $ alias is in the closure.
Wim Leers’s picture

Indentation fix.

jessebeach’s picture

I went through the patch line by line, which is surprisingly easy to do after the refactor. Most of the code is comments! And speaking of comments, I edited many of them where they varied slightly from our standard. Much of this code is left over from before the refactor.

I didn't touch any code, just comments.

In my opinion, we're ready to commit this. Any further changes should be addressed in #1989974: [meta] Clean up/improve consistency of Edit after the bulk of the refactor is in place and we can considering behavior changes in isolation.

Status: Needs review » Needs work
Issue tags: -JavaScript, -sprint, -Spark, -edit-followup

The last submitted patch, edit-create-refactor-1979784-30.patch, failed testing.

Wim Leers’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
Issue tags: +JavaScript, +sprint, +Spark, +edit-followup

The last submitted patch, edit-create-refactor-1979784-30.patch, failed testing.

nod_’s picture

Yesterday Wim and Jesse walked me through the patch and after test failures are gone and a little stray line is remove that's RTBC for me.

Wim Leers’s picture

Status: Needs work » Needs review
Wim Leers’s picture

This removes the stray line that nod_ mentions in #34 (and fixes the line above).

nod_’s picture

Status: Needs review » Reviewed & tested by the community

RTBC for me :) see #34

Gábor Hojtsy’s picture

I think it would be important to get this land sooner than later. The sheer size of it makes it too big of a target for rerolls.

alexpott’s picture

Assigned: Wim Leers » Dries

As this is removing libraries added to core and this is Dries's call.

jessebeach’s picture

Our motivations for this change are detailed in the issue summary.

Wim Leers’s picture

And before vs. after data in terms of # of bytes to load are available at #27. Number of bytes to download have been cut in half, both minified and minified + gzipped.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 8.x. Thanks!

Gábor Hojtsy’s picture

Issue tags: -sprint

Superb, thanks!

Wim Leers’s picture

Yay! :)

droplet’s picture

#1. Click quick edit
#2. Click CLOSE icon
#3. @see browser console

TypeError: b is null
[Break On This Error]

...b)}}function x(b){var c;if(s&&b in{Paste:1,Cut:1})return CKEDITOR.TRISTATE_DISAB...

ckeditor.js?v=4.1 (line 514

=============

#1. Click quick edit
#2. Click stop quick edit
#3. Click quick edit
#4. Click body text to edit
..
.
.
ERROR:

uncaught exception: The editor instance "editor1" is already attached to the provided element.
[Break On This Error]

...new CKEDITOR.dom.element(c):null},getSelectedText:function(){var a=this._.cache;...

=============

#1. empty body text
#2. no quick edit in menu

nod_’s picture

need a new issue for the bug

effulgentsia’s picture

Issue tags: -Avoid commit conflicts

This was committed, so removing the "Avoid commit conflicts" tag.

Automatically closed -- issue fixed for 2 weeks with no activity.