Photoswipe Caption: Selected source field is printed including the field label (no matter if the label is turned off in the field configuration)

Thanks, that looks like a bug in the implementation. I guess it should never ever render the label.

@thomas.frobieter exactly. I implemented that, please try if it returns the expected results.

This is kind of a breaking change, but the previous implementation doesn't make a lot of sense to me and we shouldn't make this more complicated by allowing to select a view mode etc.
From my perspective the value is what we want.

@Grevil: Please review and checked if the output runs through Twig. Otherwise, we need to ensure that the value is properly escaped for security reasons!

I added a second option, which might be more intentional, as I saw the lines before. It looks to me as if the view mode parameter was entirely forgotten before, so the field was always rendered in full?

Not yet sure how to proceed here perfectly.
a) Use MR104
b) Use MR105
c) Use MR105 and add another view mode for "field value" and rename the existing to "field in viewmode" or sth. like that.

Opinions?

@Grevil:

I'd vote for MR!104 plus
- security check (see #6)
- Removal of the strange view mode code above
- Use a common prefix like "Field value: " + human readable field name + (machine name) if not too complicated
- Set title as default value for this selection

Rendering the field seems super strange and incomplete this way, as the selected field might
- be hidden or configured differently in the default view mode
- have a label (like described in the IS)
- have complex HTML

So the label is the better solution.

Please also have a look at code changes (blame) when this was introduced and wny... the history of this change might be interesting.

Was this already existing like this in 4.x?

MR!104 seems fine, I will check the points you gave.

But we should check the original issue, this was added in.

Was this already existing like this in 4.x?

Yes.

Please review and checked if the output runs through Twig

It gets escaped based on the field provided. For example, if we use the body WYSIWYG field, and use "FULL HTML", nothing is escaped and we can "inject" scripts in the caption. If we use "Restricted HTMl" it gets escaped. Normal field values do get escaped, so I get we need additional escaping here.

Don't quite understand a few points, but please review! Should be all set now!

@Grevil: I left comments, please see.

Don't quite understand a few points

Which?

- Use a common prefix like "Field value: " + human readable field name + (machine name) if not too complicated
- Set title as default value for this selection

Rendering the field seems super strange and incomplete this way, as the selected field might
- be hidden or configured differently in the default view mode
- have a label (like described in the IS)
- have complex HTML

So the label is the better solution.

This.

Use a common prefix like "Field value: " + human readable field name + (machine name) if not too complicated

I meant the field display select options. Currently they show the field machine name. If simple, we should improve it like described Prefix + HR + MR names (see above)

Set title as default value for this selection

Separate issue now: #3393024: Make the default Caption source 'title' (not 'alt')

Rendering the field seems super strange and incomplete this way, as the selected field might
- be hidden or configured differently in the default view mode
- have a label (like described in the IS)
- have complex HTML

So the label is the better solution.

Fixed by MR!104

@Grevil: Clearer now?

yes

Ok, please review once again @Anybody!

Commented finally. Afterwards RTBC + Merge, as these are only textual changes.

Alright, tiny code adjustment, good thing tests exist! 😅

Final review through the adjusted test :)

I am going insane, with these freaking field formatter third party settings!!!!

Should be fixed now, please review!

PHPCS is complaining (and eslint but I think that's old)

And one unresolved comment.

The phpcs issue was also related to another issue, but fixed anyway!

Alright, all green now! I accidentally skipped the pipeline and executed it manually afterwards. Unfortunately the UI shows it still as "skipped", see here for the pipeline output: https://git.drupalcode.org/issue/photoswipe-3392976/-/pipelines/29389.

RTBC! :)