Problem/Motivation

Right now we use #type => html_tag, which takes #attributes => src => URL. The problem is URL becomes a single string parameter, so altering and updating the URL query parameters requires some manipulation on that string, which is painful.

Proposed resolution

Write a custom element type for iframes which specifically accepts query parameter as it's own array which can be modified before rendering. It might look something like:

'#type' => 'embed_iframe',
'#url' => 'https://www.youtube.com/embed/fdbFVWupSsw',
'#params' => [
  'autoplay' => '1',
  'start' => '100',
  'rel' => '0',
],

Remaining tasks

Validate the approach and write a patch.

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#30 2684595-easy-query-overrides-30.patch11.42 KBsam152
#30 interdiff.txt483 bytessam152
#29 2684595-easy-query-overrides-29.patch11.43 KBsam152
#29 interdiff.txt888 bytessam152
#29 interdiff.txt888 bytessam152
#27 2684595-easy-query-overrides-27.patch11.43 KBsam152
#27 interdiff.txt595 bytessam152
#21 2684595-easy-query-overrides-10.patch11.29 KBsam152
#10 interdiff.txt2.57 KBsam152
#10 2684595-easy-query-overrides-10.patch11.29 KBsam152
#9 2684595-easy-query-overrides-9.patch8.72 KBsam152
#9 interdiff.txt4.92 KBsam152
#3 2684595-easy-query-overrides-2.patch4.81 KBsam152

Comments

Sam152 created an issue. See original summary.

sam152’s picture

sam152 commented
Status: Active » Needs review

Here is an initial crack. Should only fail the YouTube tests so far.

Sadface about requiring procedural code.

sam152’s picture

sam152 commented
StatusFileSize
new 2684595-easy-query-overrides-2.patch4.81 KB

Status: Needs review » Needs work

The last submitted patch, 3: 2684595-easy-query-overrides-2.patch, failed testing.

The last submitted patch, 3: 2684595-easy-query-overrides-2.patch, failed testing.

The last submitted patch, 3: 2684595-easy-query-overrides-2.patch, failed testing.

The last submitted patch, 3: 2684595-easy-query-overrides-2.patch, failed testing.

The last submitted patch, 3: 2684595-easy-query-overrides-2.patch, failed testing.

sam152’s picture

sam152 commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt4.92 KB
new 2684595-easy-query-overrides-9.patch8.72 KB

Added some tests for the render element and the query string object.

sam152’s picture

sam152 commented
StatusFileSize
new 2684595-easy-query-overrides-10.patch11.29 KB
new interdiff.txt2.57 KB

Fixed the field output test.

The last submitted patch, 9: 2684595-easy-query-overrides-9.patch, failed testing.

The last submitted patch, 9: 2684595-easy-query-overrides-9.patch, failed testing.

The last submitted patch, 9: 2684595-easy-query-overrides-9.patch, failed testing.

The last submitted patch, 9: 2684595-easy-query-overrides-9.patch, failed testing.

The last submitted patch, 9: 2684595-easy-query-overrides-9.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 10: 2684595-easy-query-overrides-10.patch, failed testing.

The last submitted patch, 10: 2684595-easy-query-overrides-10.patch, failed testing.

The last submitted patch, 10: 2684595-easy-query-overrides-10.patch, failed testing.

The last submitted patch, 10: 2684595-easy-query-overrides-10.patch, failed testing.

The last submitted patch, 10: 2684595-easy-query-overrides-10.patch, failed testing.

sam152’s picture

sam152 commented
Status: Needs work » Needs review
StatusFileSize
new 2684595-easy-query-overrides-10.patch11.29 KB

CI error?

Status: Needs review » Needs work

The last submitted patch, 21: 2684595-easy-query-overrides-10.patch, failed testing.

The last submitted patch, 21: 2684595-easy-query-overrides-10.patch, failed testing.

The last submitted patch, 21: 2684595-easy-query-overrides-10.patch, failed testing.

The last submitted patch, 21: 2684595-easy-query-overrides-10.patch, failed testing.

The last submitted patch, 21: 2684595-easy-query-overrides-10.patch, failed testing.

sam152’s picture

sam152 commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt595 bytes
new 2684595-easy-query-overrides-27.patch11.43 KB

Wrong docblock maybe?

benjy’s picture

benjy commented 
  1. +++ b/src/Element/HttpQueryString.php
@@ -0,0 +1,70 @@
+    return (count($this->query) > 0 ? '?' : '') . http_build_query($this->query);

    return http_build_query($this->query) ?: ''; 

  2. +++ b/src/Element/VideoEmbedIFrame.php
@@ -0,0 +1,59 @@
+        [get_class($this), 'preRenderInlineFrameEmbed']

    Can also use static::class 

  3. +++ b/src/Element/VideoEmbedIFrame.php
@@ -0,0 +1,59 @@
+      $element['#query'] = new HttpQueryString($element['#query']);

    Shame this object is needed, wonder if a lazy_builder could help here to process the element. 

  4. +++ b/video_embed_field.module
@@ -0,0 +1,21 @@
+        'query' => NULL,
+        'attributes' => NULL,

    Why not default to empty arrays, same as the element info?

sam152’s picture

sam152 commented
StatusFileSize
new interdiff.txt888 bytes
new interdiff.txt888 bytes
new 2684595-easy-query-overrides-29.patch11.43 KB

1. Already discussed.
2. I like this better.
3. No idea how this works, will have to look into it.
4. Good point.

sam152’s picture

sam152 commented
StatusFileSize
new interdiff.txt483 bytes
new 2684595-easy-query-overrides-30.patch11.42 KB

More feedback.

sam152’s picture

sam152 commented

  • Sam152 committed a95963a on 8.x-1.x 
    Issue #2684595 by Sam152, benjy: Make it easier to overriden specific...
sam152’s picture

sam152 commented
Status: Needs review » Fixed

Thanks for the review!

Status: Fixed » Closed (fixed)

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