CommentFileSizeAuthor
#17 interdiff.txt746 bytesslashrsm
#17 2851512_17.patch906 bytesslashrsm
#12 entity_browser-fix_warning_none_numeric_-value-2851512-12.patch979 bytesSylvainM
#9 entity_browser-1943336-php_warning_non_numeric_value.patch625 bytesglass.dimly
#8 entity_browser-fix_warning_none_numeric_-value-2851512-8.patch866 bytesSylvainM
#4 2851512-none-numeric-value.patch1.04 KBErik Seifert
#2 2851512-none-numeric-value.patch1.03 KBErik Seifert
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Erik Seifert created an issue. See original summary.

Erik Seifert’s picture

Erik Seifert CreditAttribution: Erik Seifert commented
FileSize
2851512-none-numeric-value.patch1.03 KB
zerolab’s picture

zerolab CreditAttribution: zerolab at Torchbox commented
Status: Active » Needs work 
+++ b/src/Plugin/EntityBrowser/Display/Modal.php
@@ -108,6 +108,10 @@ class Modal extends IFrame {
+      $this->configuration['height'] - 90;

This does nothing, Did you mean $height = $this->configuration['height'] - 90?

Erik Seifert’s picture

Erik Seifert CreditAttribution: Erik Seifert commented
FileSize
2851512-none-numeric-value.patch1.04 KB

Updated patch file.

slashrsm’s picture

slashrsm CreditAttribution: slashrsm at MD Systems GmbH commented
Issue tags: +D8Media

Configuration form only allowed numerical values for width/height so the configuration value that is causing problems probably comes from some other source. Could you share the config yaml of the entity browser that is causing problems?

Erik Seifert’s picture

Erik Seifert CreditAttribution: Erik Seifert commented
Status: Needs work » Closed (works as designed)

It seems to be form an older version of entity browser allowed non numeric values. So we had non numeric values in our feature export. Could be closed.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler at bio.logis Genetic Information Management GmbH commented
Status: Closed (works as designed) » Needs work

Since #2684989: Entity Browser Responsive Modal Dialog you can actually leave the height and width configuration empty, which will lead to '' (an empty string) being saved as the configuration value. The configuration schema for height and width also uses string as its type. In this case PHP then yields the error mentioned above, when doing '' - 90.

SylvainM’s picture

SylvainM CreditAttribution: SylvainM at Axess Open Web Services commented
Status: Needs work » Needs review
FileSize
entity_browser-fix_warning_none_numeric_-value-2851512-8.patch866 bytes

With attached patch, I solve the warning

glass.dimly’s picture

glass.dimly CreditAttribution: glass.dimly commented
FileSize
entity_browser-1943336-php_warning_non_numeric_value.patch625 bytes

The patch in #8 cannot work for the error I have, which is on line 119, and the patch comes later in the code.

This is much simpler and has fixed the issue for me.

SylvainM’s picture

SylvainM CreditAttribution: SylvainM at Axess Open Web Services commented

@glass.dimly: I don't think it is a good idea, because if the height is not filled in in configuration, the height attribute will be negative

zerolab’s picture

zerolab CreditAttribution: zerolab at Torchbox for The Chartered Society of Physiotherapy commented
Status: Needs review » Needs work

@SylvainM -- similarly, for #8, it needs a check that $this->configuration['height'] is not less than 90 ;)
Otherwise, #8 LGTM

SylvainM’s picture

SylvainM CreditAttribution: SylvainM at Axess Open Web Services commented
FileSize
entity_browser-fix_warning_none_numeric_-value-2851512-12.patch979 bytes

Damned! ;-)

This is fixed with attached patch.
I added another check (is_numeric) because someone could try with percentages

SylvainM’s picture

SylvainM CreditAttribution: SylvainM at Axess Open Web Services commented
Status: Needs work » Needs review
Brian-C’s picture

Brian-C CreditAttribution: Brian-C at American Medical Association commented

Patch #12 LGTM.

sam.spinoy@gmail.com’s picture

sam.spinoy@gmail.com CreditAttribution: sam.spinoy@gmail.com at Adapt commented

Tested patch #12, works fine, allows me to add images to nodes again.

mErilainen’s picture

mErilainen CreditAttribution: mErilainen at Wunder commented
Status: Needs review » Reviewed & tested by the community

Works as advertised.

slashrsm’s picture

slashrsm CreditAttribution: slashrsm at MD Systems GmbH commented
FileSize
2851512_17.patch906 bytes
interdiff.txt746 bytes

Fixed formatting as per https://www.drupal.org/docs/develop/standards/coding-standards#linelength

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 17: 2851512_17.patch, failed testing. View results

  • slashrsm committed 4cdf5bc on 8.x-1.x 
    Issue #2851512 by Erik Seifert, SylvainM, glass.dimly, zerolab, slashrsm...
slashrsm’s picture

slashrsm CreditAttribution: slashrsm at MD Systems GmbH commented
Status: Needs work » Fixed

Test fails are not related, will fix them in a separate issue. Committed.

  • slashrsm committed 484fe31 on 8.x-2.x 
    Issue #2851512 by Erik Seifert, SylvainM, glass.dimly, zerolab, slashrsm...

Status: Fixed » Closed (fixed)

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