Create.js breaks when used with jQuery UI 1.10

[wimleers]

As mentioned on Twitter, using Create.js + jQuery UI 1.10 = breakage. So, in-place editing is currently broken in Drupal 8, because Drupal 8 sadly does not yet have JS test coverage…

For a demo, create a sandbox here: http://simplytest.me/project/drupal/dba4763dfa5589db081b53b034f43c539ea9daf2, then create an article node, then use in-place editing.

[scottgonzalez]

Let me know if you need any help with the jQuery UI update.

[bergie]

@scottgonzalez thanks for the offer! I think I have most of the issues fixed now. In both Create.js and Hallo we're using a lot of "sub-widgets", and so http://bugs.jqueryui.com/ticket/7810 broke a lot of things. I wonder if there is a cleaner way nowadays to access widget instances.

[scottgonzalez]

We're adding an instance() method to all widgets in 1.11 (it's already in master) to prevent this problem in the future. For now, using .data( widgetName ) is still the only way to access this.

[bergie]

@scottgonzalez ok, is there an easy way to see the namespace of a widget? This feels quite messy: https://github.com/bergie/create/blob/master/src/jquery.Midgard.midgardEditable.js#L327

[scottgonzalez]

Given just the name, you can't get the namespace. But you're getting the name from custom data that your'e storing, so you could put the namespace in there. Alternatively, you could proxy $.widget.bridge() and have it store the namespace (or even just the constructor) as a property of the $.fn.foo method.

[wimleers]

I tested the recent changes (thanks for those!) in Drupal 8. Summary: not quite there yet :(

1. Created fresh clone, rebuilt Create.js, copied it into Drupal (core/misc/create/create-editonly.js)
2. Clear browser caches, try… and crash. Due to the s/Create/Midgard/ namespace changes. Easily fixed.
3. Try again. New problem:
4. Debugged this, and it's precisely https://github.com/bergie/create/blob/master/src/jquery.Midgard.midgardEditable.js#L327's Midgard- prefix that @bergie cited above that was causing problems: As you can see: no Midgard-SOMETHING, but createWidgetName… I looked, but couldn't find anything in Drupal's Create.js integration code that could cause this. I could be wrong though.
5. I "fixed" this by temporarily removing the Midgard- prefix. Solves the problems in the screenshots above … but, now we're back to square one: same problem as the one at the very top of this issue!

Drupal 8 patch at http://drupal.org/node/1960612#comment-7253930, test it in a sandbox via this link: http://simplytest.me/project/drupal/8.x?patch[]=http://drupal.org/files/create_js_jquery_ui_1_10-1960612-1.patch

[bergie]

@wimleers the issue I discussed with @scottgonzales above is that now to retrieve a widget instance, we need to do a data call with namespace-widgetname instead of just widgetname as it was previously.

Having Midgard hardcoded there is not optimal. Instead, the namespace should be in config, with maybe a Midgard default.

I suppose this broke because your widgets use the Drupal namespace.

[wimleers]

Indeed, I switched them from the DrupalEditEditor namespace to the Midgard namespace and now I was able to get past those problems. But … then what is the point of using namespaces at all? It seems like we made incorrect assumptions about how namespaces are intended to be used?

All of a sudden I have to retrieve Create's Storage Widget via data('DrupalCreateStorage') instead of data('createStorage') though — why is that?

With that second problem out of the way, it seems everything is working as before again. See the interdiff for the updated Drupal 8 patch: http://drupal.org/node/1960612#comment-7255754.

[scottgonzalez]

You could temporarily (until 1.11 ships with the instance() method) add a method to make this work. Do the editors inherit from a common base?

[scottgonzalez]

All of a sudden I have to retrieve Create's Storage Widget via data('DrupalCreateStorage') instead of data('createStorage') though — why is that?

Because in 1.9 we deprecated the use of just the short name and in 1.10 we removed the back compat.

[wimleers]

@scottgonzalez Sorry for not being sufficiently clear. It's fine that it has to change, I just don't understand how "Drupal" ended up in that name. AFAICT we don't feed the string "Drupal" into Create.js anywhere. So I think it's a question that only @bergie can answer :)

[bergie]

@wimleers I'm assuming the custom widgets in Edit are registered to the Drupal namespace to have events named like 'drupaleditablechanged' instead of the default Midgard ones

[wimleers]

@bergie the only namespace we set (again, AFAICT) is this one:

      this.$el.createStorage({
        vie: this.vie,
        editableNs: 'createeditable'
      });

[wimleers]

@scottgonzalez Also, AFAICT it is impossible to be compatible with both jQuery UI 1.10 and 1.8 without introducing version checks?

@bergie If the above is true, then what is your plan for Create.js? Only support jQuery UI 1.10, or also earlier versions?

[scottgonzalez]

Also, AFAICT it is impossible to be compatible with both jQuery UI 1.10 and 1.8 without introducing version checks?

It's always possible to be compatible, it's just a matter of how much work you need to do.

For this specific case, it's as simple as:

var instance = elem.data( "ns-widget" ) || elem.data( "widget" );

If you had a ton of checks like this, you could create a helper method.

[wimleers]

@bergie Is what @scottgonzalez suggested above acceptable? In general, what are your thoughts on being compatible with multiple jQuery UI versions?

[bergie]

@wimleers yeah, that sounds like a plan. We should add namespace to the widget configs (for example when configuring editing widgets

[wimleers]

@bergie So, how do you want to move forward with @scottgonzalez' suggestion?

[bergie]

I just pushed some fixes that should help in running against both jQuery UI 1.10 and earlier. All widgets used with Create still have to be in the Midgard namespace, though.

@wimleers and @flack, could you verify?

[flack]

I did a quick test a the JS errors seem to be gone now. But there still seems to be some bug in there, when I create a new item, not all fields get saved properly (I have the impression that only the last field gets saved, but I could be wrong). Editing works fine, though, so only POST is affected

[flack]

P.S.: Also, the minified version is missing from the repo: Is that deliberate or some build system snafu?

[bergie]

@flack removal of the minified version was on purpose.

As for creating items, could you debug that a bit more? The raw JSON sent to server would help, as would contents of vie.entities

[flack]

the raw post data looks like this:

<http://openpsa2.org/createphp/content> "content"
<http://purl.org/dc/terms/partOf> ["<http://dev-ccb/midcom-...e1be79a1ee855a94f694f6>"]
<http://purl.org/dc/terms/title> "heading"
@subject "_:bnode143"
@type "<http://contentcontrol-berlin.de/what-items>"

The value "content" gets saved properly, the value "heading" is lost. About vie entities I can't say much, because I don't know what it is and where to find it :-)

[bergie]

@flack given that both attributes are in the POST data, I suspect the issue is on the server end

[dbu]

jquery was updated a couple of times since then. still relevant?

[flack]

on my end, I'm running on 1.10.3 for a while now without any issues, but I obviously can't speak for everybody