Problem/Motivation

Drupal core's Standard profile ships CKEditor configured to allow uploading images by the click of a CKEditor button. Those images are currently not exported, therefore cannot be imported either.

Steps to reproduce

  1. Install core with the Standard profile.
  2. Create a Basic page node, with an image uploaded using the CKEditor button.
  3. Export the node.
  4. Reinstall the site, making sure that the uploaded file was deleted (or use a completely different environment for importing).
  5. Import the zip.

Expected behavior: image uploaded using the CKEditor button gets exported, therefore gets imported as well.

Actual behavior (as of 1.4.6): image uploaded using the CKEditor button is NOT exported, therefore cannot be imported either.

Proposed resolution

Add code that supports this.

Remaining tasks

Review, merge, release.

User interface changes

None.

API changes

None. Exporting file entities also became possible.

Data model changes

None.

CommentFileSizeAuthor
#2 single_content_sync-3420464-2.patch5.63 KBboobaa

Issue fork single_content_sync-3420464

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Boobaa created an issue. See original summary.

boobaa’s picture

boobaa
he/him
commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new single_content_sync-3420464-2.patch5.63 KB

Attached patch provides this feature, based on already-existing examples.

mxr576’s picture

mxr576
Primary language Hungarian
Location Hungary
commented 
$this->privateTempStore->get('export.assets')

What is going to flush out things stored in this storage after the export is done? I see that whatever is stored there gets overridden every time. By the way, due to this user-specific but shared storage, isn't there a slight chance for unexpected behaviors when the same user executes multiple exports at the same time?

boobaa’s picture

boobaa
he/him
commented

@mxr576: While your comment is asking a proper question, it does not belong to this particular patch/issue. The race condition you mentioned is also present without this patch, as this user-specific but shared storage is also used elsewhere already during the export process. Because of that, I think a different issue should handle that problem – if at all, because I also think that this is quite a corner case: I just cannot really imagine myself executing another export while the first one also started by myself is still running. IOW, this is theoretically possible for sure, but practically? I'm not that sure.

mxr576’s picture

mxr576
Primary language Hungarian
Location Hungary
commented

roger :) thanks for the detailed answer

rprager’s picture

rprager commented

The patch in #2 didn't work for me. I received an error when trying to export. I changed line 45 from

public function mapBaseFieldsValues(array $values, FieldableEntityInterface $entity): array {

to

public function mapBaseFieldsValues(array $values): array {

and that worked for me.

boobaa opened merge request !115

boobaa’s picture

boobaa
he/him
commented
Issue summary: View changes

Checked the report from @rprager in #6 but I could not reproduce it (using the latest 1.4.x branch, which corresponds to the 1.4.6 version as of now): I have created a node and used CKEditor to upload an image to its body, then exported the node. The only log entry I got was about node creation, and the resulting zip did contain the image I have uploaded via CKEditor.

So I have converted the patch in #2 into a merge request as-is.

mxr576’s picture

mxr576
Primary language Hungarian
Location Hungary
commented

Hiding the old patch.

mxr576’s picture

mxr576
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Needs work

Left a suggestion.

I also checked and it seems to me that the modified code has no test coverage, so there is nothing to extend. Covering TextField processor with tests should be a dedicated issue/effort.

mxr576’s picture

mxr576
Primary language Hungarian
Location Hungary
commented
Status: Needs work » Reviewed & tested by the community
nginex’s picture

nginex commented
Assigned: Unassigned » nginex

morvaim made their first commit to this issue’s fork.

cobblestone.consulting’s picture

cobblestone.consulting commented

This works perfectly if the data-entity-type and data-entity-uuid attributes are added to the -img- tag. However, in my install, when uploading a new image via imce (3.1.2 or 3.1.3) and then adding that image in CKEditor, I am not seeing any data attributes.

Is there some additional configuration in imce or ckeditor required to get the data attributes to be included?

nginex’s picture

nginex commented

@cobblestone.consulting do you use CKEditor 4 or 5? Did you try with full html (no tag attributes limit)?

nginex’s picture

nginex commented
Status: Reviewed & tested by the community » Needs work

I realized that FileAsset can be refactored, so we keep the same logic in a single place File.php processor. FileAsset needs to simply export file entity then. Created a new branch to handle it

nginex opened merge request !158

nginex’s picture

nginex commented
Assigned: nginex » Unassigned
Status: Needs work » Needs review

Provided rework, so now the logic is in one place, File plugin, FileAsset plugin now contains only the field-related logic. Backward compatibility has been provided.

nginex’s picture

nginex commented
Status: Needs review » Fixed

Going to include this to the release. Tested with old exported content and new one. It works like a charm. Any feedback is welcome

  • nginex committed b3cb8610 on 1.4.x 
    [#3420464] Rework for FileAsset plugin to better support file entity...

Status: Fixed » Closed (fixed)

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