[regression] Text field's label is duplicated in the value when edited & saved using Quick Edit

This doesn't happen when using formatted text, because then the original (unfiltered/unformatted) text is retrieved from the server.

This is a regression caused by #2559955: Ensure that Quick Edit relies less on the structure of the HTML.

Is this only happening with long text fields, or just fields with labels above?

So #2559955: Ensure that Quick Edit relies less on the structure of the HTML changed this:

var $fieldItems = this.$el.find('.field__item');

to:

var $fieldItems = this.$el.find('.quickedit-field');

Now if we look at the HTML structure, then it totally makes sense why the bug reported in this issue is happening:

#6: just with text fields in general. I'm pretty sure this also happens with non-long text fields.

In fact, I retract #4, because this should also happen for formatted text fields as long as they don't use any filters of the types \Drupal\filter\Plugin\FilterInterface::TYPE_TRANSFORM_REVERSIBLE or \Drupal\filter\Plugin\FilterInterface::TYPE_TRANSFORM_IRREVERSIBLE.

I just tried this with a "Text (formatted, long, with summary)", "Text (formatted)", and "Text (plain, long)". It only happens with the plain text field. Not sure why, when structurally they all appear to be the same. I'm not seeing any difference between them in the markup.

#9: see the bottom part in #4. Try it with a text format that doesn't have any filters. Then you should be able to reproduce it there too.

I tried with a new text format, with no filters. I cannot reproduce. I either get the hovering toolbar, or the popup with the field data and the select box for choosing a different text format. The plain text field is the only one that edits directly in page and grabs the label. I'm going to try with other field types.

Doesn't work with a number field, probably for obvious reasons. It has that increase/decrease value widget. But it did work with an email field, which also does not use a text format.

#11: Hm, I'll need to check why that is then. Thanks for testing that!

#12: It can only ever be the plain & rich text fields, because those are the ones that have custom in-place editors. Everything else falls back to the form-based in-place editor, and so it couldn't possibly happen there, because forms don't use contenteditable.

I just perform round of testing and able to replicate the same thing. I am taking this under my assignment and provide the update ASAP.

This is still an open issue

Just ran into this. When deleting the value text you can also delete the label. It then saves the label as the value. Very weird to see that happening.

This patch fixes the issue by keeping the `textElement` variable to scope the whole field (presumably for managing the interaction?) but only storing the text value of the `.content-field` child.

Edit: the previous fix was missing some places where the DOM selector needed to be updated. I submit here an updated patch for 8.4.x as well as a patch for 8.3.x.

A nicer patch would probably create a new property to store the actual content field separately from the field wrapper but I’m not familiar enough with the core conventions to propose something properly named.

My bad, please disregard the previous patches for the moment: they assume the presence of CSS classes that are not part of the stable theme. I’ll update my patches later to reflect this.

So here’s an actual working patch tested on the latest 8.4 dev branch. Again the coding style is probably not the best but as this actually works, I hope some core maintainer can improve on it and merge that for an 8.4.x release?

I can confirm #23 is solving the issue for classy themes, but the problem is that we start running in circles. The error was introduced in #2559955: Ensure that Quick Edit relies less on the structure of the HTML and the patch more or less is reverting it.

Re-rolling this patch to work on newer Drupal versions.

Even though I understand it's not desirable to have specific markup on this, I guess this patched version will work by default with both scenarios (either having labels or not) with most websites that uses Drupal default structure.

I just ran into this issue, so updating the version to 8.7.x.

I tested this on a site using Bartik and it didn't seem to fix the issue.

Bartik for example uses the class field__label for the field label. For this reason #27 seems to fail on Bartik.

We have the same issue with ckeditor fields - with the following in core/modules/ckeditor/js/ckeditor.js:

element.setAttribute('contentEditable', 'true');
return !!CKEDITOR.inline(element, settings);

I tried to apply the same reasonning as in the patch - but then it messed everything up.

Additionnally, bootstrap barrio is also using field__label - so this doesn't seem the right approach to the issue.

There should probably be a config setting for the class name (re: #30). This patch is an alternative to #27 where replaces dashes with underscore specific to using a theme set up for this format.

Re-rolled patch. please review.

Re-rolled patch #35. Attached interdiff for same.

Hi @Gauravmahlawat
Thanks for the patch.
Verified and tested patch #38.
Patch applied successfully and looks good to me.

Testing Steps:
# Goto: Appearance -> Apply Seven theme
# Add a field "Summary" (long text, plain) to the Article content type
# Create a new Article node and go to that page
# Click the pencil icon on the node content and choose "Quick edit"
# Click the summary field (Notice how the Field label becomes editable too), make a change, and click "Save"
# Notice that the field label has been duplicated in the field content: the field content now starts with "Summary"

Expected Results:
# After applying the patch, the Field label should not appear as a duplicate.

Actual Results:
# Currently, the field label has been duplicated in the field content: the field content now starts with "Summary"

Please refer attached screenshots for the same.
Looks good to me.
Can be a move to RTBC.

I think we should still add test coverage to make sure that this regression doesn't happen again.

Due to Quickedit being moved out of Drupal Core and into a Contrib Module, moving this issue to the Contrib Module queue.

Just tested patch from #38 on site with 9.2.11:

- patch applies but doesnt work.

Theme is bootstrap based. I am using Simpler Quickedit module (as without it i can't get QE to work on user fields). And this is on a user plain text field with the label Above the field. When the label is inline, as soon as i edit the label is embedded into the edit area. So i guess technically i could remove it and then when i save it isnt re-added (like with label set as Above); but obviously not the right answer.

I've added a test case for this and hope it makes sense. My approach is a bit different, I look for the .field__label class (instead of both .field__label and .field__item) and add contenteditable="false" to those. If such a label was found, the user cannot edit it, to hopefully prevent some confusion. Label contents are stripped from the saved value. If there's no dedicated label, everything stays the same as before.

The general problem is that we need some way to figure out what's the label and what's the actual value we want edited. This is however highly dependent on the theme :( So for now, you'd need to add that field__label class to your templates.

Follow-up patch that fixes restoring the original field after changes were made and discarded.
We should probably also add tests for "the label is displayed as expected after discarding changes".

Fix whitespace, thanks text editor... Also added an interdiff.