🇺🇸United States @hyperlinked

I've had a really rough time dealing with CORS for quite some time with a mobile app that I developed using the Ionic Framework. After countless hours of frustration, I finally realized what the issue was and I haven't seen this called out so hopefully this saves someone some grief.

If you use a protocol name that is not named, the entire line of your configuration that uses that invalid protocol will be discarded. I don't know if it was always the case or if in previous versions of Drupal it was more tolerant and only ignored the origins using unrecognized protocols. As of Drupal 10.3, it causes the Access-Control-Allow-Origin header to be entirely suppressed.

An example version of my services.yml is below, but in short the problem was that one of my allowedOrigins was capacitor://localhost. That's the equivalent of http://localhost from inside an Ionic app. In order to support that protocol, it needs to be added to the filter_protocols parameter.

parameters:
  filter_protocols:
    - http
    - https
    - capacitor

  cors.config:
    enabled: true
    allowedHeaders: [ 'x-csrf-token','authorization','content-type','accept','origin','x-requested-with', 'access-control-allow-origin','x-allowed-header']
    allowedMethods: [ '*' ]
    allowedOrigins: [ 'http://localhost:8100', 'capacitor://localhost', 'https://myapp.firebase.com' ]
    exposedHeaders: true
    maxAge: false
    supportsCredentials: true

@catch would there be any harm in the forseeable future with fixing this via the workaround in #72 🐛 10.3 upgrade now missing status-message theme suggestions Postponed for a non-contrib legacy theme?

Ryan, setting your cache bins to null in your settings disables page caching, which is basically like setting cache.max-age = 0 to everything on the site. That's handy for the dev site, but you won't want that on your production site. If that was what you had resorted to, you don't want to do that as your workaround.

OK, so I wasn't the only one. That's exactly what I did as well. I guess the workaround in #17 wasn't complete. If anyone is having issues with messages getting cached and frozen, you'll need to use the code in #72 until there's a proper fix for this.

Creating a custom preprocess hook for system_messages_block as shown in #17 worked for me, but I ran into caching problems with the messages block. The message output would get cached and it would display the cached message every time.

I had to set the cache max-age to 0 for this to actually be usable. Did anyone else encounter this too?

I ended up needing to do this:

use Drupal\Core\Render\Element\StatusMessages;

function yourtheme_preprocess_block__system_messages_block(&$variables) {
  $variables['content'] = StatusMessages::renderMessages();
  $variables['#cache']['max-age'] = 0;
}

I'll defer judgement on that since this is my first time encountering this exact scenario with compatibility issues for a drop-in replacement in Core.

I'd presume it'd be the nice thing to do support both until it's actually dropped in Drupal 12. There may be other modules that continue to reference the old methods and those may break Commerce Checkout if only the new methods are supported. Case in point is Mail Login 4.x and that's the module that brought me here.

I'm not familiar with using a union type to address this scenario to provide options. How would that work?

FYI, if you're seeing an error message that looks something like the following, you may be dealing with a module that has conflicts between the old UserAuthInterface and the new UserAuthenticationInterface:
"TypeError: Drupal\commerce_checkout\Plugin\Commerce\CheckoutPane\Login::__construct(): Argument #8 ($user_auth) must be of type Drupal\user\UserAuthInterface, Drupal\mail_login\AuthDecorator given"

If you had a problem with Mail Login and your search led you here, you need to downgrade to Mail Login 3.x.

hyperlinked → created an issue.

hyperlinked → made their first commit to this issue’s fork.

Thanks for flagging this priyanshu9311858.

Thanks for your quick help govind_giri_goswami!

Thanks for catching that. I'm going to assign this to Performance.

I've confirmed that is indeed a problem and can also confirm that the patch works against the 6.0.3 build. In my case, this bug happened because I needed to execute a View programmatically very very very eary in the request process to evaluate whether the view should be rendered or bypassed.

At such an early phase of the request process, the route object has not been built yet so if the view being executed uses Better Exposed Filters, the route object won't be available.

I don't understand the exact mechanics, but executing the default view programmatically worked fine. It's only if I set a current display for the View that I would run into this error where Better Exposed Filters tries to access hasOption() on a null route object.

In my case and in the case of this reported issue, the View is only being executed and won't be rendered to the user so it's inconsequential to bypass the hasOption() call.

This was happening to me too and in my situation there was a very easy workaround. I normally run "drush config-import" first before running "drush updatedb".

I just ran drush updatedb first so that permissions that would be removed when I updated the config wouldn't be removed yet.

Thanks James! I've reviewed the latest commits and I can confirm that I'm able to have my lead data sent to Marketo when a user does something to trigger a lead sync that doesn't involve a program name.

I can also confirm that a Webform submission with a form ID is properly submitted and received by Marketo.

I can't confirm if everything that uses a program name is working properly as I've never used one of those, but I can confirm that the program name when supplied now affects the execution path of the API request whereas before, the program name was always discarded.

BTW, I initially reviewed the wrong branch and I committed some similar changes to the 3.x branch. That commit can be ignored. Everything is in this one already. I'm bad at Gitlab.

hyperlinked → created an issue.

Updated patch with corrected patch indexing level.

I closed my merge request after realizing that more substantial adjustments would be needed, but these changes could have much farther reaching implications for this module that needs the guidance of someone closer to the source. For now, I'm going to post a patch and explain what I think needs to be done. This patch will work against 3.0 and the 3.0-dev releases.

In Drupal\marketo_ma\Service\MarketoMaServiceApiClient the syncLead method actually corresponds to the push.json endpoint of the Marketo REST API. There is no method corresponding to the leads.json endpoint which is what syncLead should be aligned with. If it doesn't create havoc, syncLead needs to be renamed to pushLead and a new syncLead method should be created.

Note: I will be referring to the REST API endpoints and not any Marketo MA method names to avoid any confusion.

Why is this important? Originally the programName variable was optional for push.json. This made leads.json and push.json interchangeable for single lead actions. At some point, Marketo began throwing errors if you used push.json without a programName, but there doesn't appear to be any part of Marketo MA that's actually setting a programName properly.

The only part of the code where programName is used is in the Webform handler for MarketoMA. You can enter the programName, but it's never used because the Webform Handler bypasses the updateLead methods that would post lead info to leads.json and push.json. Furthermore, the Marketo MA webform handler will collect programName, but if you use programName, you can supply a form ID. Without a form ID, the submission will fail from the Drupal side.

In my attached patch, I've made it so that we have:
syncLead -> leads.json
pushLead -> push.json

Test coverage and interface updates are included.

This assumption that the authorization header may not exist should also be applied further to the content-type header that comes in the next block of code. Because of this, I'm proposing an alternate patch rolled against the 2.x-dev branch.

I've made a small adjustment to ensure that we don't perform a trim on a null value by using a null coalescing operator. I think this is also closer to the coding style of the original author.

This change in this patch will pass an empty value directly to trim if no authorization or content-type header is passed.

I've verified that the associateLead action has been fixed as of the 3.0 branch stable release. This likely was fixed in an earlier release, but I was cleaning up my patches today and checked to see if the patch I rolled for this issue was still relevant. It no longer is because the issue is no longer active.

I tested it by visiting some random pages on my site in a fresh browser environment. I then created an account and upon logging in for the first time and saving my account settings to save my new password, a new lead was created in Marketo for my test user. Upon inspecting the person data in Marketo, it showed actions taken while my activity was still anonymous and the New Person activity type appears in mid-stream at the point in which I signed in to my new account.

@HeikkiY, good idea. Done. Someone please review so we can get this merged and not end up accidentally duplicating more effort.

Here's one single patch that will combine all of the previously contributed Drupal 10 compatibility patches into one patch file. This patch will apply cleanly against the 8.x-1.x-dev branch (aka the 8.x-1.5 branch).

There was still another issue after patch #26 from @ajeet_kumar. Thie issue was described by HeikkiY in #28. The file FileDownloadDownloadController.php was using a depricated Unicode library to read Mime headers. The native Drupal File::getMimeType works just fine.

I've combined the numerous patches into a commit and added my fix to the mimeHeaderEncode() bug. I may have inadvertently created some extra branches in this issue fork. If someone is able to delete them, please do so. My commits are made against the main branch of file_download-3297221.

I'm requesting a review.

hyperlinked → made their first commit to this issue’s fork.

Ajeet, I'm confused why you opened a new issue for D10 compatibility when there's already the one open here:
https://www.drupal.org/project/file_download/issues/3297221 📌 Automated Drupal 10 compatibility fixes RTBC

You're the main contributor on that thread. It looks to me this issue thread is both a duplicate of that one and also less complete. If this one is duplicate, we should close it as a duplicate to reduce the potential for confusion.

I've resolved the unrelated issues that were blocking me from successfully completing a remote site authorization and I can now confirm that patch #16 is working for me and resolves all of my issues. I'm using the authorization code grant with refresh tokens.

Nevermind, #16 works. I didn't have the two required vendor packages installed. After requiring bshaffer/oauth2-server-php and bshaffer/oauth2-server-httpfoundation-bridge, #16 is working. I haven't been able to confirm that I'm able to login yet as I'm having some other unrelated issues with the login process.

Patch #16 isn't completely working for me on 10.1.5. When I clear the cache I get the following error:

Interface "OAuth2\OpenID\Storage\AuthorizationCodeInterface" not found

@webengr and @simohell you can also add this issue repo as a custom git repo to composer.json and install the specific branch you want. So your repositories would look something like:

"repositories": {
        "commerce_checkout_order_fields": {
            "type": "git",
            "url": "git@git.drupal.org:issue/commerce_checkout_order_fields-3336589.git"
        },
        "drupal": {
            "type": "composer",
            "url": "https://packages.drupal.org/8",
            "exclude": ["drupal/file_download","drupal/commerce_checkout_order_fields","drupal/filter_twig"]
        }

    },

And you would then specifically install the Drupal 10 compatibility branch like so:
composer require drupal/commerce_checkout_order_fields:dev-3336589-drupal-10-support

Disregard my last comment about merging the commit into the 8.x-2.x branch. That was the wrong commit. All of the user submitted patches are incorrect. The one that needs to be merged is the one from Project Update Bot #2 as verified by @webdrips.

That "error" message you're getting appears to be a ChatGPT response, which suggests the issue is with the way your prompt was written.

This isn't a module error response:

I cannot extract keywords without the text enclosed within the triple backticks. Please provide the text.

Can you post the prompt that you're using?

And requesting a review of my commit to change the hook_update_N processing.

I've installed the issue fork and tested it to verify that Augmentor is able to work without requiring CKeditor 4 being invoked on the entity edit form. It works as expected both if there is no CKEditor at all on the form or if CKeditor 5 is enabled instead.

I've also made a couple of commits to this issue fork.

I moved the hook_update_N function into an *.install file in accordance to typical Drupal best practices.
I made enabling the augmentor_ckeditor4 module dependent upon having CKeditor 4 already installed to prevent users who do not want CKeditor 4 getting it enabled by accident.

hyperlinked → made their first commit to this issue’s fork.

This is a duplicate of this issue here:
https://www.drupal.org/project/augmentor/issues/3358019 ✨ Separate Ckeditor 4 in a submodule Fixed

It's caused because Augmentor has a dependency on CKeditor 4 and if CKeditor 4 isn't the chosen editor loaded for that entity form, Ajax requests will fail with that console error.

I do believe this warrants the use of the once method. I can't see any reason why this block of code has to be invoked numerous times. I've been testing this on a complex site and it was running more than five redundant API calls after the initial results were processed.

I've created an issue fork to address this.

hyperlinked → made their first commit to this issue’s fork.

It appears that not only does Augmentor only support CKeditor 4. It actually requires CKeditor 4 to be selected on the pages on which there is an Augmentor edit form widget. I tried numerous variations on a clean install.

I tried numerous variations in a clean install:
CKeditor 4 enabled, but not selected as the editor for a formatted textarea element.
CKeditor 4 and 5 enabled, but CKeditor 5 selected as the editor.
CKeditor 4 enabled, but no editor selected.
CKeditor not installed at all.
CKeditor 4 enabled and selected as the editor for at least one textarea.

The only variation that works is the last one. If CKeditor 4 is not loaded on a page that has an Augmentor widget, the following JavaScript error is thrown when clicking on the Augmentor widget button:

[Error] ReferenceError: Can't find variable: CKEDITOR

@eleonel, confirmed. I was wrong about the formatter breaking the Augmentor field and widget. I'm doing clean installs and something else was getting in the way. The augmentor would hang or abort if CKEditor4 was not used on the page. That's for another issue.

Whoops, I may have spoken too soon. @naveenvalecha, your PR fixed the original issue, but now the Augmentor doesn't work anymore when the default formatter is changed to field_augmentor_formatter AugmentorItem Field Type Plugin.

Was it wrong to be using the field token value module for the default formatter?

Hi @naveenvalecha confirming that your PR fixed the issue.

I also had this exact same issue and it was a problem on a clean install.

If the problem that @murrayw is experiencing is the same as mine, it's because the Augmentor AI module doesn't have the drupal/field_token_value module listed as a dependency in the *.info file. Once I added this module and to my project and enabled it, this error went away.

One way this bug happens is if you have your site within a subdirectory of your webroot directory.

Let's say you installed your site at https://www.drupal9.foo/mysite where mysite is actually a directory of your webroot directory. The way redirects are typically specified in Drupal is from the directory of your installation so your may be getting redirects to /admin/content. Drupal is recognizing that as external because technically speaking /admin/content is actually "https://www.drupal9.foo/admin/content". Using that as redirect will get you the external redirect error.

If you can instead set the redirect to "/mysite/admin/content", the error will go away as it's recognized as being within the same site. 

At least this is what I've encountered with sites within subdirectories. I've also triggered this error in ways that were entirely my own fault due to sloppy coding, but this is the only version of the redirect error that I've encountered that might actually be a bug. 

@mjgruta, go into the product types configurations and check the "Allow more than one variation" checkbox. That will move the SKU and Variant Title off of the primary product entity form and allow you to perform a bulk edit.

There's also a workaround you can use in a pinch that works if any field doesn't play well with the patch.

Select the SKU and Price fields and enter a value in for each field. Now uncheck those fields as fields you want to edit. Whatever you just entered will not be applied. They'll remain filled in so you'll be able to submit the form, but the values will be ignored.