Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Loading alohaeditor (without invoking it) should not mutate the dom #752

Closed
karger opened this Issue · 15 comments

4 participants

@karger

Simply including the aloha editor as a script in your document causes the dom to mutate. The class "aloha-mozilla" is added to the body (at least in firefox). divs to hold the sidebar, the aloha ui context, aloha toolbar, and pasteContainer are appended to the dom. None of this is needs to or should be done before .aloha() is actually invoked; it prevents other scripts from accessing a "clean" copy of the dom before launching aloha. Also, mahalo() should remove all these elements, in order to guarantee a clean copy of the whole dom for later use.

Except at times when these elements are actually being displayed, there is no reason for them to be in the dom at all---they can work just as well as dom fragments!

This is related to the cleanup issues discussed in issue 459 .

@karger

The other problem with putting this stuff in the dom (when aloha is inactive) is that any tool that removes them will prevent aloha from working.

@deliminator

Although it is probably just as you say - they can work just as well as dom fragments - it's more convenient to just put the classes/elements in the dom and not worry about adding/removing them whenever an editable is activated/deactivated.

Some of the elements are from third party libraries. For example "jstree-marker" and "vakata-contextmenu".

I think it would be a non-trivial effort to inject/eject these elements only when needed. I would therefore like to ask what problem you have, that could be solved by investing this effort. I assume it's not just about a maintaining "clean" DOM that looks nicer for your users?

@karger

It's certainly not about looks for the user, since the elements are invisible. But it's about being predictable. If they aren't cleaned up, they'll get saved. If they get saved, then some time in the future, someone's overly-general selector may end up materializing some of these unwanted elements. Or, it may break someone's assumption about the dom---e.g., that the body has only one

child.

It's easy to say that good programmers won't make the kind if mistakes that would be impacted by these leftovers, but there are a lot of bad programmers out there. For example, me: I'm writing an editor that needs to be able to "reinitialize the body" and my first approach to doing that was simply to empty() the body and add my own content. This broke aloha.

Perhaps worst, it isn't clear what's going to happen if I open a "partially mahalo'd" document later in aloha----will it break? How can I know?

I recognize that it's always more convenient not to bother, but given how your system pairs up alohas and mahalos, it would seem pretty straigtforward to require that everything that is created (by an aloha) is removed by the corresponding mahalo.

As a developer working on top, I can of course do my own cleanup. But the problem is that I can't know all your code or how its changing. So, for example, I am already manually removing the leftover tags I mentioned above/ However, you've just mentioned some others I've never seen---"jstree-marker" and "vakata-contextmenu"---so didn't know I need to remove. It's never going to be possible to do a good cleanup job from the outside; that's why I hope you will tackle it internally.

@karger

So with perfect timing, this issue just tricked me into several hours of debugging. Roughly speaking, I wanted to aloha-edit the entire docoment, as in $('body').aloha. But aloha doesn't work on the body (not sure why) so I invoked $('body').children().aloha(). But as we discussed above, some of those elements are aloha elements. It turns out that invoking aloha() on the aloha elements breaks aloha: the toolbar stops being dragable. (Also, the menu items on the toolbar, and the pastecontainer, become editable, which is how I eventually discovered what was wrong).

Now that I understand the problem, the right solution is for me to filter out the aloha elements before invoking aloha() on the rest of the div. Unfortunately, as we've been discussing, since aloha fills the dom during script load, before I have time to look at the page, and since the aloha-added elements have no one distinctive class, there is no way for me to clearly identify the aloha elements so that I can avoid breaking them!

@karger

I've placed a demonstration of the problem at http://people.csail.mit.edu/karger/Aloha/too-much-aloha.html

@karger

I've also discovered that detaching and then reattaching the aloha elements from the dom breaks aloha. Which forces me to very carefully keep those nodes (which I shouldn't have to know about at all) in place as I modify everything around them. Too much work and definitely violating the abstraction barrier!

@deliminator

#589

Making the body editable seems to be not supported for exactly that reason - you need to have a place to put the UI elements, and you can't put them in an editable (ok, it could possibly work somehow, but an editable DOM doesn't work exactly like a non-editable DOM due to browser issues, so it's just a bad idea in my opinion).

There is the ui/context module. The idea behind that module was that all UI elements of Aloha get appended to a single child of body. It is already used in one or two places, but not everywhere. 3rd party libraries (jqueryui, jstree) don't provide control over where elements are appended.

@karger

Yes, I understand why it would be bad to make body editable.
But consider the natural solution:
$('body').wrapInner('

'); $('#stuff-to-edit').aloha();
This looks like it should work---it puts everything I want to edit inside a div that can be made editable.
Unfortunately, since aloha mutates the dom while loading, I have no opportunity to grab just the stuff that I want. By the time my code can run and wrap the body, there are already aloha elements in it that will be pulled into my div (which will break aloha). I have to identify every element that aloha might add and explicitly leave it out of my wrapper. If in the future you change your code to introduce another element, then my code will break.

@wimleers

Interesting issue :)

Did you try using Aloha.deferInit()? That's what we're using in Drupal (initially mostly to be able to do Aloha.settings = Drupal.settings.aloha.settings).

See drupal.aloha.js for an example.

@karger

deferInit looks like it could be a useful trick. But the current setup, requiring you to listen for a suitable event and then invoke deferInit at just the right time, seems complicated. Wouldn't it be simpler to put an "intitalize: false" property into aloha.settings?

@wimleers

You can call deferInit() on document load. It's this simple:

$.ready(function() {
  Aloha.deferInit();
});
@karger

And then how do you trigger init to occur when you want it?

@wimleers

Hm? This is the init?

@karger

Wait, does Aloha.deferInit() mean "now please carry out the init that has been deferred" or does it mean "please don't init aloha yet"? If the former, then how do I tell aloha not to init at the usual time, but to wait for me to call deferInit? If the latter, then after I've told it not to init (by calling deferInit), how do I tell it when it is time to init?

@deliminator

Here is how it works

// defers init
Aloha.deferInit = true;
<script src="aloha.js" ...></script>
...
// calls init
Aloha.deferInit();
$(...).aloha();

We decided to do away with this process and do lazy initialization. In other words, in the future aloha will be initialized in the first call to aloha().

@evo42 evo42 closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.