Committed to 2.x and 3.x. Thanks!
kim.pepper → created an issue.
I reverted the autowiring of SearchApiConverter as the BC logic in there was too complex. Created a follow up for that 📌 [PP-1] Autowire SearchApiConverter service Postponed
kim.pepper → created an issue. See original summary → .
kim.pepper → created an issue.
kim.pepper → created an issue.
I don't think it qualifies as a bug if you manually put in a url with missing params.
Updated the issue summary and added a change record.
Oops! Not sure how to best manage multiple analyzers.
I realize this will conflict with 📌 [pp-3] Split up and refactor system_requirements() into OOP hooks Active is therefore postponed on that, but wanted to make a start to see what changes are needed.
Working on this.
📌 Add value objects to represent the return of hook_requirements Needs work is blocked on this.
I pushed up my work in progress in case it's useful later.
Damn. I wish I saw your comment before I started converting system_requirements() 😓
Looks like drupal_requirements_url() is the only caller of drupal_current_script_url() so both of these can be deprecated. I don't think putting them on the RequirementSeverity enum is the right place. These functions are used by the install script to pass query params around.
Rebased on 11.x
smustgrave → credited kim.pepper → .
Reviewed the changes. Looks nice and simple.
kim.pepper → created an issue.
kim.pepper → created an issue.
I think this can be closed as 📌 Create enums for RequirementSeverity and RequirementPhase Active went in which deprecates the REQUIREMENT_* constants.
Yeah i don't think we want to do any of that on the Drupal side.
Manually tested and confirmed adding the synonyms filter to the default analyzer fixes this issue.
Committed to 2.x and 3.x. Thanks!
Committed to 2.x and 3.x. Thanks!
Should just be 'verify' => false according to the docs. https://docs.guzzlephp.org/en/stable/request-options.html#verify
As for the exception in #2, that just looks like an authentication error?
FYI the docs say quotes for array keys in array shapes aren't needed but I added them anyway https://phpstan.org/writing-php-code/phpdoc-types#array-shapes
Thanks for the reviews. Resolved all threads.
Do you mean constructor params? This may just be the limitation of the underlying call to stream_wrapper_register() which just takes a class name. See https://www.php.net/manual/en/function.stream-wrapper-register.php
Adding a related issue saying we should prevent duplicates ✨ Drupal should prevent uploading of duplicates for managed files Postponed: needs info
Actually this is a feature request.
Ran the test-only job and confirmed it fails.
Should we close this issue in favour of 📌 Create enums for RequirementSeverity and RequirementPhase Active ?
Still a few unsresolved comments on the MR
Rebased on 11.x
I also switch the file install config to replace_whitespace: true. I'm not sure this is sufficient as a default or whether we want to go the extra step and enable it via an update hook. This will be disruptive as per #2
Feedback has been resolved and all tests are green 🟢. Back to NR.
Looks like \Drupal\package_manager\ValidationResult was created using int values in
✨
Add Alpha level Experimental Package Manager module
Needs review
before this could get commited. 😞 Created a follow up
📌
[PP-1] ValidationResult should use RequirementSeverity enum
Active
kim.pepper → created an issue.
Added a CR about the new setting.
I'm working on rebasing this. Now most of the hooks are converted to OOP we can see what is left over.
kim.pepper → changed the visibility of the branch 11.x to hidden.
greggles → credited kim.pepper → .
greggles → credited kim.pepper → .
Rebased on 11.x.
I think this might get more attention if it were under theme system.
Looks like permissions aren't set correctly in your file system. Did you follow the setup instructions here? https://www.drupal.org/docs/administering-a-drupal-site/security-in-drup... →
Changes should be against 3.x and need to be in a MR to trigger the CI pipeline.
Looks like some legit test fails.
Rebased on 11.x
kim.pepper → made their first commit to this issue’s fork.
Updated title.
Updated IS to reflect latest proposed resolution.
Added a default mime type constant of \Drupal\Core\File\MimeType\MimeTypeGuesser::DEFAULT_MIME_TYPE with value 'application/octet-stream' and use that instead of empty string.
Hiding some older patches and MRs.
kim.pepper → changed the visibility of the branch 3469280-drupalfileiconmimetypes-doesnt-handle to hidden.
kim.pepper → changed the visibility of the branch 3469280-File-mime-type-null to hidden.
Changing from a bug to a feature request.
I think this is a performance improvement rather than a bug so changing to a task.
Tagging as needs issue summary update to add in the template, and rename title to replace drupal_move_uploaded_file() which no longer exists.
According to
https://www.drupal.org/about/core/policies/core-dependency-policies/depe... →
we need framework and release manager signoff on adding a new dependency on psr/clock. 🤷
Rebased on 11.x Tests green again.
TBH I don't have a strong opinion on which characters should be included or excluded. I'll happily defer to those with a stronger security background. I _do_ think removing whitespace will be disruptive to site owners.
Based on #10 I think we can RTBC this now.
Left some suggestions.
I think we agreed on stripping special characters. It was more about which characters to strip. I have a feeling stripping whitespace might be contentious.
I had to look into why unset($transaction) works and it's because we have a \Drupal\Core\Database\Transaction::__destruct() method that calls
$this->transactionManager()->unpile()
Looking at the solution were all options taken?
Do you mean are all the special characters included?
Test fail looks unrelated
Drupal\Tests\big_pipe\FunctionalJavascript\BigPipeRegressionTest::testPlaceholderHtmlEdgeCases
WebDriver\Exception\UnknownError: unknown error: session deleted because of page crash
from unknown error: cannot determine loading status
from tab crashed
(Session info: chrome-headless-shell=123.0.6312.58)
(Driver info: chromedriver=123.0.6312.58 (6b4b19e9dfbb93aa414dc045bd445287977d8d7a-refs/branch-heads/6312_46@{#3}),platform=Linux 5.10.234-225.921.amzn2.x86_64 x86_64)
kim.pepper → changed the visibility of the branch 3519726-fix-incorrect-mimetype-service to hidden.
kim.pepper → changed the visibility of the branch 10.4.x to hidden.
kim.pepper → changed the visibility of the branch 11.x to hidden.
Created a MR from the commit in #2
This bug only exists in 10.4.x as it triggers a deprecation that was removed in 11.0.x.
Updated the title and IS.
kim.pepper → made their first commit to this issue’s fork.
Reviewed the code changes and agree with moving legacy tests. Unfortunately a test-only run doesn't not fail to confirm the bug is fixed as the code that would fail has been moved to a new test class. Not sure if it's worth it pursuing?
Addressed feedback and updated the IS.
I think we can create a new requirements check for the config setting. However, there was some pushback (#44_) on showing warnings on a standard install, so we need to be sure this is considered.
@mondrake created https://www.drupal.org/project/sophron/issues/3519605 📌 SophronMimeTypeMapFactory should just override MimeTypeMapFactory::doCreateMap() Active
kim.pepper → created an issue.
Tests back to green.
Rebased on 11.x
I re-ran the pipeline a couple of times to see if it was a random failure, but still getting the fails.
Rebased on 11.x
Back to NR for the feedback from @acbramley
Rebased on 11.x and added docs to default.settings.php
This feature request was posted against D7 which is no longer supported. If there is a feature request that would apply to Drupal 11+ then please feel free to update the Issue Summary and describe the details of that request.
Attempted to write a test for this, but couldn't work out why the invalid operation was not being passed through to \Drupal\Core\StreamWrapper\LocalStream::stream_lock() correctly. 🤔
Updated the IS saying we propose to return FALSE for unknown operations.