Provide support for masking HTML tags while editing

We discussed this requirement and it seems clear that TMGMT need to provide support for masking while editing.

So if we have an Editor that need tag masking, it means
- The text area will not show the original values. Instead it will show the masked values.
- On submission, the masking is reverted.

For this, two hooks should be provided:
- One that is triggered before we output the values in the form.
- One that is triggered when the values are taken from form state and applied to the job/data item.
This allows easy implementation of the ckeditor integration.

Mind that both hooks will receive the alterable variable with the single data text. And additionally, they will get job item / job context so they know what they have to do.

They will get *two* contexts. $data_item and $job_item.

Yeah sorry i thought after our discussion, the description was incomplete.

Now that most of the discussion about the data mangling hooks is here, why not create a separate issue to discuss the mapping?

Here is a starting patch, to see if this approach works good. Still need to discuss about considering to add a context to distinguish source from translation and if the unmask hook is placed in the right place.
And of course still need test coverage.

1. +++ b/src/Form/JobItemForm.php
   @@ -346,14 +347,19 @@ class JobItemForm extends TmgmtFormBase {
   +          $text = $data_item['#translation']['#text'];
   

   Hmm, we extract $text and allow altering it, but it is later not used?! It should be attached to $data IMHO otherwise it won't have any effect?

2. +++ b/src/Form/JobItemForm.php
   @@ -346,14 +347,19 @@ class JobItemForm extends TmgmtFormBase {
   +          \Drupal::moduleHandler()->alter('tmgmt_data_item_text_input', $text, $data_item, $item);
   
   @@ -962,10 +968,16 @@ class JobItemForm extends TmgmtFormBase {
   +    $text = NULL;
   ...
   +      \Drupal::moduleHandler()->alter('tmgmt_data_item_text_output', $text, $data_item, $this->entity);
   

   I think the two hooks need documentation in .api file.

Done + starting test coverage.

Next step will be to implement some real masking as part of @sasanikolic's project as a pull request on github against his project, so we have a real case before committing the patch. Will do this on monday.

Looks pretty nice.

+++ b/src/Form/JobItemForm.php
@@ -962,10 +958,16 @@ class JobItemForm extends TmgmtFormBase {
+      \Drupal::moduleHandler()->alter('tmgmt_data_item_text_output', $text, $data_item, $this->entity);

I think we should always trigger this alter...
The case where translation is empty could be important to initialise it with empty segment-only tags or filled with source text and special indication "needs translation".

I've just added a check where it could be possible to handle the tags thing for this empty case. Now I'll create a PR in the github project to provide a real example.

Started with some regex to mask/unmask the hardcoded b- and img-tags. Created PR here. Still work in progress.

+++ b/src/Entity/Job.php
@@ -708,12 +708,7 @@ class Job extends ContentEntityBase implements EntityOwnerInterface, JobInterfac
-      $this->submitted();
...
+    $this->getTranslatorPlugin()->requestTranslation($this);

Accidental revert of 9d74ad9

Sorry, forgot to update my feature branch :P

OK, discussed and decided...

We learned with the segmenter and the editors that we should think in source + translation pairs.
Thus the hook should also operate on the pair level.

See JobItemForm::reviewFormElement()

        // Build source and translation areas.
        $item_element = $this->buildSource($item_element, $data_item, $rows, $form_state);
        $item_element = $this->buildTranslation($item_element, $data_item, $rows, $form_state, $is_preliminary);

The hook should fire before buildSource and buildTranslation, and the modified masked text should be passed into these methods as a new argument.

The submission only needs to operate on the translation. Unmasking source is not needed.
And then, we can finally commit this. :-)

Done. Improved some comments and code.

Just a small improvement :p

Hmm we should never attach temporary processed values to an entity. Anyone in the chain could trigger a save and thus accidentally persist the temporary value.

Instead create separate properties, variables as derivatives and pass them forward separately.

Here the intention was / is to add a $text argument to the buildXYZ methods (and yes, change their declaration!)

Alright, done.

Almost there, just a bit of cleanup and comment improvements.

1. +++ b/src/Form/JobItemForm.php
   @@ -329,31 +329,32 @@ class JobItemForm extends TmgmtFormBase {
   +        $original_text = is_array($value['translation']) ? $value['translation']['value'] : $value['translation'];
   +        $text = $original_text;
   +        // Initialise the empty translation with empty segment-only tags.
   +        if ($text == '') {
   +          $text = '';
            }
   

   As discussed, simplify to $text = ...; no $original_text needed.

2. +++ b/src/Form/JobItemForm.php
   @@ -329,31 +329,32 @@ class JobItemForm extends TmgmtFormBase {
   +        $contexts = [$data_item, $this->entity];
   +        \Drupal::moduleHandler()->alter('tmgmt_data_item_text_input', $text, $contexts);
   +
   

   $contetx needs to use array keys ('data_item' => $data_item, 'job_item' => $this->entity)

3. +++ b/src/Form/JobItemForm.php
   @@ -475,9 +476,17 @@ class JobItemForm extends TmgmtFormBase {
   +        // Mask HTML-tag of source's text and translation's text, if available.
   +        $source_text = $data_item['#text'];
   +        // If translation is empty, initialize it with empty segment-only tags.
   +        // @todo: Add context to show that this is a translation.
   +        $translation_text = isset($data_item['#translation']['#text']) ? $data_item['#translation']['#text'] : '';
   +        $contexts = [$data_item, $this->entity];
   +        \Drupal::moduleHandler()->alter('tmgmt_data_item_text_output', $source_text, $translation_text, $contexts);
   

   First comment: Allow other modules to change the source and translation text, for example to mask HTML tags.

   Remove the second comment, that is logic that belongs in tmgmt_ckeditor (for now).

   And the @todo can be removed.

   (the code is fine)

4. +++ b/tmgmt.api.php
   @@ -297,3 +298,30 @@ function hook_tmgmt_job_after_request_translation(JobInterface $job) {
   + * @param array $context
   + *   An array containing $data_item and $job_item contexts.
   + */
   

   list the keys with their content. See hook_entity_view_display_alter()

Done as suggested above.

Thanks, now I'm happy with it. Committed.