facebookarchive / facebook-instant-articles-sdk-php

The Facebook Instant Articles SDK for PHP provides a native interface for creating and publishing Instant Articles.
https://instantarticles.fb.com/
Other
232 stars 148 forks source link

Unable to use ConstantGetter with InteractiveRule #335

Closed m4olivei closed 6 years ago

m4olivei commented 6 years ago

Steps required to reproduce the problem

  1. Define a Transfomer rule to match embeds and apply InteractiveRule with constant width and height. eg.
    $rules[] = [
    'class' => InteractiveRule::class,
    'selector' => '//div[@class="embed-code-other"][iframe[contains(@src, "soundcloud.com")]]',
    'properties' => [
      InteractiveRule::PROPERTY_IFRAME => [
        'type' => 'string',
        'selector' => 'iframe',
        'attribute' => 'src',
      ],
      InteractiveRule::PROPERTY_WIDTH => [
        'type' => 'constant',
        'value' => '320',
      ],
      InteractiveRule::PROPERTY_HEIGHT => [
        'type' => 'constant',
        'value' => '160',
      ],
    ],
    ];
  2. Run the transform against some HTML that includes:
    
    <div class="embed-code-other">
    <iframe width="100%" height="300" scrolling="no" frameborder="no" allow="autoplay" src="https://w.soundcloud.com/player/?url=https%3A//api.soundcloud.com/tracks/461019930&color=%23ff5500&auto_play=false&hide_related=false&show_comments=true&show_user=true&show_reposts=false&show_teaser=true&visual=true"></iframe>
    </div>

# Expected Result

* You get an `op-interactive` with width = 320 and height = 160

# Actual Result

* When running the transform, you get the following error:

InvalidArgumentException: Method expects this value ----[

 /var/www/vendor/facebook/facebook-instant-articles-sdk-php/src/Facebook/InstantArticles/Validators/Type.php:174:string '320' (length=3) 
]---- to be one of the types ====[
 /var/www/vendor/facebook/facebook-instant-articles-sdk-php/src/Facebook/InstantArticles/Validators/Type.php:179:string 'INTEGER' (length=7) 
]==== in Facebook\InstantArticles\Validators\Type::throwException() (line 182 of /var/www/vendor/facebook/facebook-instant-articles-sdk-php/src/Facebook/InstantArticles/Validators/Type.php).

# Additional comments
The problem here appears to be that the ConstantGetter [requires a string](https://github.com/facebook/facebook-instant-articles-sdk-php/blob/master/src/Facebook/InstantArticles/Transformer/Getters/ConstantGetter.php#L32) value, whereas the Interactive element [requires an integer](https://github.com/facebook/facebook-instant-articles-sdk-php/blob/master/src/Facebook/InstantArticles/Elements/Interactive.php#L89).

I'm currently working around this by defining my own ConstantGetter extending the Facebook ConstantGetter:

<?php

namespace Drupal\ls_fb_instant_articles\Facebook\InstantArticles\Transformer\Getters;

use Facebook\InstantArticles\Transformer\Getters\ConstantGetter as FacebookConstantGetter; use Facebook\InstantArticles\Validators\Type;

/**

}

everton-rosario commented 6 years ago

Thanks for that @m4olivei . Do you think if we add that type enforcing thing into our base code would be enough? You can send that piece of code as PR.

m4olivei commented 6 years ago

@everton-rosario Yeah totally, just making ConstantGetter more liberal like my override class would do it. Sure, I'll file a quick PR.

m4olivei commented 6 years ago

@everton-rosario I don't have time to dig in to a point where I could add a test, so I'll leave it there for now, feel free to add a test if you think it needs it.

everton-rosario commented 6 years ago

Sounds good @m4olivei