Add title to iframe for A11y reasons (8.x)

This issue is a duplicate, but is marked for Drupal 7, so I'll just mark them as Related.

I reviewed the patch, and it seems mostly OK, but I ... Given that there is already a proposed Drupal 7 patch on the other issue, I think it would be a good idea if the Drupal 8 patch were more similar, especially where the Drupal 7 patch is I think a little better. Here are some things I think need to be fixed in this patch:

a) Call the new config element iframe_title, not just title. [It seems like "title" is quite ambiguous. Could be the title of the section the maps are in, the title of a link, etc.]

b) In the settings form, the D7 patch has:

+  $element['iframe_title'] = array(
+    '#title' => t('Title of iframe for embedded map'),
+    '#type' => 'textfield',
+    '#default_value' => $settings['iframe_title'],
+    '#description' => t("The embedded map is in an iframe HTML tag, which should have a title attribute for screen readers (not shown on the page). Use [address] to insert the address text in the title."),
+  );

This seems better than the settings form in this patch.

c) The D7 patch has the ability to embed the address as part of the title (see patch on #2902178-15: Add title attribute to iframe (7.x).

d) In the patch here, the iframe title hasn't been added to the settings summary. It should be.

e) The schema YML file is missing a newline at the end in this patch.

@Eric Heydrich

Thanks for working on this ... yes A11y is a difficult thing to do correctly.
I think you are correct and title should be corrected. so +1 on the issue in general.
[ it is good to have another pair of eyes looking at this.]

I have a minor nit and then a catchability issue.

looking at the so life cycle of 'iframe_title'

a) Defined as a string in the schema.
b) Defaults to empty string in SimpleGMapFormatter:defaultSettings()

The inconsistency then is that the default theme/twig variable iframe_title defined in simple_gmap_theme() should also be a empty string and not NULL. ( that way conceptually it is a string all the way through )

Here are my step to reproduce the catchability issue - found during manual testing.

Adjust the formatter form to be "[address]" so I could test that if a hacker inputs a malicious SCRIPT tag as the title then sanitization will mangle the SCRIPT tag into the harmless form ( this test passes - as expected but hey just checking )

The catchability issue comes from the fact that when I returned to the formatter form and changed the iframe title to something stupid like 'socks' the catchability issue becomes apparent.. instead of socks the munged script tag was still displayed.

So we need a cache invalidation strategy - in the general sense as actually any change to the formatter config( say width ) needs to trigger a cache invalidation.

As to the way forward here I am flexible there are many valid ways to proceed.

Create a cache bubbling issue ? ( probably best dealt with separately - I will try and find time to do that next week )
That is a module wide issue and a bug that has existed for a long time ... so I am happy to move this issue to RTBC once the NULL nit is corrected.

@jhodgdon in the interests of efficiency .. if you are happy .. move this to RTBC and I will fix the NULL nit on commit.

Is that cache problem true for any change to the settings? I would have thought that Drupal Core would take care of invalidating the cache if any settings on any field formatters were changed. It seems crazy that each field formatter would need to take care of it. ?!? Weird.

Anyway, the patch is fine from my point of view. Seems like you've tested it, and are ready for RTBC so I'll go ahead and change the status. I need to do something with the 7.x patch too...

Thank you both for taking the time to look at this...

Spawned a few side issues and fixed the NULL issue on commit.

@jhodgdon.. sorry for seeming a little rude... you questions in #11 have not gone unnoticed ...
I will answer them directly in #2931580: Cache invalidation stratergy

No worries, I wasn't feeling bad about this. :)