Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
I'm getting several JShrink related error messages concerning JShrink:
- continue in break should target the while loop directly
- strpos() doesn't longer accept non-string needles
These are already discussed & fixed upstream in the official JShrink library: https://github.com/tedious/JShrink/pull/78 .
Comment | File | Size | Author |
---|---|---|---|
#3 | advagg-replace_continue_with_break-3068558-3.patch | 1.03 KB | rp7 |
Comments
Comment #2
rp7 CreditAttribution: rp7 for Government of Flanders commentedComment #3
rp7 CreditAttribution: rp7 for Government of Flanders commentedPatch attached applies the fixes that were applied for the JShrink library @ https://github.com/tedious/JShrink/commit/21254058dc3ce6aba6bef458cff4bf... .
Comment #4
rp7 CreditAttribution: rp7 for Government of Flanders commentedComment #5
zrhoffman CreditAttribution: zrhoffman as a volunteer commentedI can reproduce the issue in a fresh install using PHP 7.3 and have verified rp7's patch in #3. `Drupal\Tests\advagg_js_minify\Kernel\Asset\JsMinifierUnitTest` fails before the patch and succeeds afterwards:
Note that the above test only covers the `continue` within `switch` bug. In order to reproduce the "`strpos()` with `false` as needle" bug at advagg_js_minify/jshrink.inc#L226 , one of the unminified JavaScript files provided by Drupal\Tests\advagg_js_minify\Kernel\Asset\JsMinifierUnitTest::providerTestMinification() (drupal.js, ajax.js, and ToolbarVisualView.js) would need to end with
)<newline>
instead of;<newline>
(or add an additional JavaScript file to test). Once that is done, and if advagg_js_minify/jshrink.inc#L269 is patched from #3 but advagg_js_minify/jshrink.inc#L226 is not, the `strpos()` with `false` as needle fix can be verified:Comment #6
acbramley CreditAttribution: acbramley at PreviousNext commentedCan confirm #5, I have triggered automated tests on the patch as well.
Comment #7
thallesThanks everyone, looks good to me!
Comment #9
thalles