🇺🇸United States @jeffschuler

MR !8 is working as well for my purpose of defining a query-based endpoint param.

Sorry @immaculatexavier!
I totally missed that you had already made this a MR. :-p

I struggled for awhile trying to figure out how to specify that a param for a custom REST resource should be in the query.

I was able to do so using the patch from #15. (#14 worked for my purposes too.)

After applying the patch, I modified my @RestResource annotation to include the info about the query param like this:

/**
 * Provides a REST resource.
 *
 * @RestResource(
 *   id = "my_rest_resource",
 *   label = @Translation("My REST Reource"),
 *   uri_paths = {
 *     "create" = "/api/v1/foo/{path_param_1}/bar/{path_param_2}"
 *   },
 *   route_parameters = {
 *     "POST" = {
 *       "my_query_param" = {
 *         "name" = "my_query_param",
 *         "in" = "query",
 *         "type" = "string",
 *         "description" = "My custom query param in addition to the path params",
 *         "required" = true,
 *       }
 *     }
 *   }
 * )
 */

Setting status to Needs work for somebody to take @anybody's advice in turning this into a MR.

Thanks for the nudge, @itamair.
8.x-1.3 created.

@tostinni: The patch in #64 on the latest 4.x-dev (and no other patches) is generally working for us inside nested paragraphs.

For example, we have a multi-value paragraph reference field that can reference a paragraph that has another multi-value paragraph reference field within. Inside those nested paragraphs there's a select field that has conditional fields visibility control over a few other fields; for various values of the select, other fields within the same nested paragraph are shown & hidden. Some of those fields are required (from within the paragraph type config). When those required fields are hidden (and empty), they do not stop the form from saving. When they are shown, they are required, as designed.

The exception to this is a required Media field within that nested paragraph. It does not interrupt the form from saving when it's hidden (good) but it also does not enforce its requirement when shown (bad). I brought this up in another issue #2902164 comment #140 🐛 Controlled-by fields inside a Paragraph don't work Needs work . I don't want to clutter this issue... only in case that's the problem you're experiencing.

Maybe you can describe your content structure and exactly what's not working?

Re-roll of #59 (with fix) for latest 4.x-dev.

Thanks @saurav-drupal-dev!

Just to clarify (& confirm): the patch alone doesn't remove back-to-site button for you, correct?
Only if-and-when you also add some custom styling?

If so, and if you do deem it RTBC, please change issue status to RTBC.

Thanks!

@_renify_: Mind summarizing your changes?
Here's the interdiff between patches #20 & #23.
We've been using the patch in #20 successfully for a year now.

jeffschuler → changed the visibility of the branch gin-3345842-3345842-remove-the-back to hidden.

The addition/modification of classes here in order to enable the removal of Back to site via CSS is useful.

The change in JS to remove the element entirely if escapeAdminPath is NULL not as clear. There is already logic for determining whether Back to site / etc. is added in gin_preprocess_breadcrumb(). Additional conditional logic for that deserves to be there (instead of JS) if possible.

And, as evidenced by Sascha's comments in #5, the question of whether to remove the element entirely is a lot more debate-able.

Incoming commit to MR!231 to remove those JS changes and constrain this issue to just allow styling of the link.

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

Ah, I see that the changes in the MR allow me to style that item directly, so in my CSS overrides I can just hide it:

.gin-breadcrumb__item--back-to-site {
  display: none !important;
}

Sorry, I'm a still dumdum when it comes to d.o./gitlab integration. My intent was to merge the latest gin 8.x-3.x into the issue branch after resolving conflicts manually, but it looks like it has created a new MR.

At any rate these changes are also not removing the Back to Site button for me.

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

Makes sense.
Thanks @paulmartin84!

Thanks @dmudie! This was really helpful.

I still wonder if there's a cleaner way to do this.

Committed to 8.x-1.x.
Thanks @paul121, @m.stenta, and @shyam-sawhney!!

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

Thanks @malcomio!

Agreed that this project is bundling itamair's fork of geoPHP (it sounds like you've identified that your phayes version is coming from elsewhere?), but the exception handling is more than welcome.

Merged into 1.x.

jeffschuler → created an issue.

The $entity parameter is the entity your computed field is attached to.
To get the entity type or content type (bundle), try:
$entity->getEntityType();
$entity->bundle();

If your entity is a node, get its title like this:
$entity->getTitle();

Check out Working with teh Entity API → or the Node class docs.

No prob; just want to see this issue continue forward!

@sidgrafix your patch in #120 and the patch in #114 are not identical.

For example, the patch in #114 has this change:

-    $this->assertSession()->statusCodeEquals(404);
+    $this->assertResponse(404, 'Deleted redirect properly clears the internal page cache.');

while your patch does not.

The #114 patch removes this line:

-    $this->assertSession()->pageTextContains('No URL redirects available.');

but yours doesn't:

     $this->assertSession()->pageTextContains('No URL redirects available.');

The #114 patch calls drupalPostForm:

+    $this->drupalPostForm('node/' . $node->id() . '/edit', ['path[0][alias]' => '/node_test_alias_updated'], t('Save'));

and yours calls drupalPostFormForm:

+    $this->drupalPostFormForm('node/' . $node->id() . '/edit', ['path[0][alias]' => '/node_test_alias_updated'], t('Save'));

...

@sidgrafix: it doesn't appear your patch is a simple reroll ("the same as #114, applied to latest version"). There are a number of material differences. Are the other changes intentional (to get this working) or an accident?

Here is #114 rerolled for the current 8.x-1.x, with the one rejected hunk fixed, (plus a diff of these two patches for verification).

Leaving this as Needs Work, as @azinck's recommendations in #106 are still not addressed.

Keep the return after finding an external URL, otherwise the path will continue to be parsed and (depending on the structure of the URL) not always end up including the $redirects_trace (from, to, and status) information.

Reverted back to the original comment because I believe it's more informative and correct than what it was recently changed to.

@vhin0210 is there a reason to add the patches from 🐛 RouterPathTranslatorSubscriber uses wrong param when resolving the url Active and 🐛 Unable to resolve path on path with fragments Needs review to this?

Mixing issues generally makes the individual issues a lot more challenging to test, fix, and get merged by the maintainer.

We're experiencing this too, just as you lay out in the "steps to reproduce", and your changes fix the issue.

I don't know the internals here very well, but this comment in BasicAuth::challengeException():

  // 2. Have the 'config:user.role.anonymous' cache tag, because the only
  //    reason this 401 would no longer be a 401 is if permissions for the
  //    'anonymous' role change, causing the cache tag to be invalidated.

...does seem to leave out the possibility that a 401 would no longer be a 401 because changing an entity status to Published gives the anonymous user access.

Here are your changes as a patch against 11.x-dev. That patch is applying and working in 10.2.2 as well.

Specifying the "ALL" union type made my D7->D10 migration's counts actually meaningful. This is still a lingering issue.

jeffschuler → created an issue.

This gets a little weirder.

With internal:, the filename is removed but the query string is not urlEncoded.

print_r(Url::fromUri('internal:' . $url)->setAbsolute()->toString());

Result: http://default/sites/default/files/styles/large/public?itok=L25Jd2Zr

But with base:, while the filename persists, the query string is urlEncoded, which will break image embeds:

print_r(Url::fromUri('base:' . $url)->setAbsolute()->toString());

Result: http://default/sites/default/files/styles/large/public/images/test.jpeg%3Fitok%3DL25Jd2Zr

This has been made essentially moot by 📌 Automated Drupal 10 compatibility fixes Fixed .
Closing.

I just had this happen, and noticed the filename only gets removed in URLs with a query string.

It looks like the removal happens here:

$url = Url::fromUserInput($url)->setAbsolute()->toString();

specifically, when fromUri() (called by fromUserInput()) gets the internal: prefix that fromUserInput() adds, it removes the filename.

Test with a simple example using @mlncn's URL:

$url = '/sites/default/files/styles/large/public/images/test.jpeg?itok=L25Jd2Zr';
print_r(Url::fromUserInput($url)->setAbsolute()->toString());

The output is

http://MYSITE/sites/default/files/styles/large/public?itok=L25Jd2Zr

I note that the docs for Url::fromUri say:

"For user input that may correspond to a Drupal route, use internal: for the scheme. For paths that are known not to be handled by the Drupal routing system (such as static files), use base: for the scheme to get a link relative to the Drupal base path (like the HTML element)."

We should be able to do this instead:

$url = Url::fromUri('base:' . $url)->setAbsolute()->toString();

This has fixed the issue for me and I'm continuing to test it.

I'm not sure, BTW, why it was only failing when there was a query string present.

@Daniel Korte. Cool. I thought it would be easier for @podarok to review and commit ✨ Allow choice of attributes to process Needs review separately since these issues accomplish different things, but let's see what he thinks.

@Daniel Korte : that would definitely be more DRY - and ultimately might also solve a few other issues like 🐛 Do not convert Relative URLs beginning with // (v2.2.5) Needs review and 🐛 Part of the path is lost after image path conversion Needs review ...

But one reason is that transformRootRelativeUrlsToAbsolute() uses a fixed set of $urlAttributes, and I think it would be useful if this module were to ✨ Allow choice of attributes to process Needs review .

Ideally transformRootRelativeUrlsToAbsolute() would allow choosing params to include/exclude, but I'm not sure it's worth trying to change core for this use case.

There's another issue in ParseDates with the break; after generating new repeat values. It breaks out of the loop completely, meaning that additional values don't get processed at all.

This for me resulted in a bunch of " [warning] A non-numeric value encountered ParseDates.php:341" errors.

Updated patch to fix this.

Alternatively, as a patch...

See MR

jeffschuler → created an issue.

One more try. Sorry for the spam.

Missed the one that matters most. Fix and simplification.

Using the patch in #38 the "limit" end date of a recurrence is being specified with time in the string, and therefore not showing up in the UI – where only a date is allowed.

It looks, from smart date data that's been input manually (non-migrated), like the rule DB column does include HHMMSS ('\THis') in the UNTIL string but the limit column does not.

i.e.

Should be:
rule: RRULE:FREQ=DAILY;INTERVAL=1;UNTIL=2017-01-08T235900
limit: UNTIL=2017-01-08

But actually is migrated as:
rule: RRULE:FREQ=DAILY;INTERVAL=1;UNTIL=2017-01-08T235900
limit: UNTIL=2017-01-08T235900

Here's an update of the patch in #38.

Looks like @nightlife2008 had this issue too, with a similar solution in #32.

Experiencing this too. I think it started when we began using the patch/MR in 🐛 Can only intentionally re-render an entity with references 20 times Needs work to address that issue. Rob230: using that patch too?

@ad0z: thank you.

After some debugging, I see that the reason the validation callback wasn't being called is because of this change that I made in patch #9 on 🐛 If required, all cells need not be full Needs review :

- if ($form_state->getTriggeringElement()) {
- $element['#element_validate'][] = [$this, 'validateTablefield'];
- }
+ $element['#element_validate'][] = [$this, 'validateTablefield'];

Some more digging to do...

Couldn't get hook_element_info_alter() to work, but was able to set a validator in hook_field_widget_WIDGET_TYPE_form_alter() (our tablefield is inside a paragraph) such that when the form is submitted, the number of columns are checked and a form error is thrown if above our max.

However, I want to validate and set the error on the submission of the AJAX rebuild functionality itself ~ when the Rebuild button is clicked. (I don't want the user to be able to rebuild their table above max rows and then not figure it out until the submit the whole node.)

Seems like this is because the $element['tablefield']['rebuild']['rebuild'] sub-form is built in the processTableField() function and doesn't yet exist when form alters happen.

I tried setting an #after_rebuild hook on the form, but $element['tablefield']['rebuild'] still wasn't set, though $element['tablefield']['#rebuild'] was true...

Thanks @ad0z! We'll give this a try and I'll re-open if we have issues.

jeffschuler → created an issue.

Yes, please.

jeffschuler → created an issue.

@Wim Leers your amount of productive output deserves a few mistakes. At least it wasn't causing any problems!

Thanks all for seeing this through.

Note that depending on the format's date/time separator an extra space could be introduced, like:

"Weekly on Monday and Friday , 10:00 a.m.–1:00 p.m."

... because it's being added for the predetermined " at" date/time join.

I am doing some post-processing to remove spaces before commas, but this should probably be fixed upstream in the solution here. Let's see if @mandclu accepts this idea in the first place and then proceed from there. :)

A simple solution to add end time that is working for our needs.

It maintains the existing "at ..." text if there is no end time, but otherwise uses the "date/time" join string from the format options.

jeffschuler → created an issue.

Hmm, maybe that is a bug I haven't noticed yet.
Is that 🐛 Unable to change resize after initial save Fixed ? Or another issue?

jeffschuler → created an issue.

Also found my way here because I didn't see "Limit allowed HTML tags and correct faulty HTML" as being required.

I had a few issues with this:

1. $form_state->getTriggeringElement() often wasn't set when I saved a node (with a tablefield inside a paragraph), so the validateTablefield() function wasn't being run. I removed that conditional from inside formElement() and changed it to use $form_state->isSubmitted()) inside validateTablefield().
2. When the table was reorder-able, the weight element inside the row was being counted as a value, so an empty table was still being saved. So we check for that.
3. All sub-fields of the tablefield were being highlighted as the error. I pass the table sub-field to setError(), since that's what we're checking has values.

To be clear, the issue is load('compact') in here:

  public function buildRow(array $instance) {
    // Get format settings.
    // @todo make the choice of format configurable?
    $format = $this->entityTypeManager
      ->getStorage('smart_date_format')
      ->load('compact');

And that @todo is relevant.

jeffschuler → created an issue.

@thatguy: That makes sense. And it works properly with the file field as the controlled-by?

@TornatoreMDM:

For the Undefined index error, it's probably necessary to repeat the check from line 537: if (isset($element_to_set_error['#title'])) { before the statement a few lines down where you're getting the error: $title = $element_to_set_error['#title'];

As for the issue where it's complaining that fields that actually have values are still required, I suspect it may be something similar to what I dealt with in #140 with media fields. There was a sub-field of the media field that didn't have a value even though the field itself did, and that was triggering the error. It sounds like the dependent field (that you're displaying and requiring) is a paragraph (another complex field that has sub-fields)? – So there might be something more general we need to do for checking whether fields are set... I added a bunch of dpm() statements to see what was triggering the form error.

Yes @eahonet, #140 is only to solve an additional issue and only applies on top of #138.

#138 is the patch you want for the primary issue here.

Sorry for any confusion and thanks for the review!

@borisson_: I understand why you're requesting this but I don't believe it's a bug that deserves tests and it doesn't result in incorrect output. It's just a logical error that results in unnecessary over-processing.

This:

foreach (static::$uriAttributes as $attr) {

loops over each of the attributes in this array:

protected static $uriAttributes = ['href', 'poster', 'src', 'cite', 'data', 'action', 'formaction', 'srcset', 'about'];

For each of these attributes the current code is looking for all instances of that attribute, and then also looking for all instances of srcset, and operating on the value of each srcset it finds.

So ... for every attribute ... we'll parse and operate on every srcset needlessly. We should only parse for and operate on each srcset once.

Weird. You said "some image paths will be filtered out". Can you try to identify which paths? ... What are the conditions where this happens?

The code in your patch is kind of re-doing functionality that setAbsolute() already handles, so it shouldn't really be necessary.

Maybe you can drop in some debug dpm()s, like on $matches and $url inside the preg_replace callback to see at exactly which point you're losing part of the path.

Something of an edge case but nonetheless an issue:

(You shouldn't need this smaller, additional patch unless you're having

#required<code> problems with media fields inside your paragraphs.)

If you're running into the problems described in 
            
              
              
              🐛
              Hidden required field is still required
                Active
              
             & 
            
              
              
              📌
              Support field turning hidden by condition to loose its required status
                Needs work
              
            , where a field set to <code>required

that's being hidden by conditional_fields is still incorrectly being required – and therefore causing form validation errors because it's empty...

and to solve this you instead make your fields optional and use conditional_fields to explicitly set them to required (with the same criteria that makes them visible)...

and one of those dependent fields is a core Media field with an image...

and it's in a paragraph...

then you might be running into the issue I am, which is that when that Media field is visible and required, even when an image has been chosen from the media library and so that field has a value, it's still being marked as empty.

It appears that the media_library_selection subfield of the media field is being caught as empty by conditional_fields in dependentValidate().

I don't see this happening with Media fields not in a paragraph, so I'm just including my extra patch to mitigate this here (which gets applied on top of #138) in case it's useful to anyone else, and in case it helps illuminate anything else regarding this issue.

In dropping dpm()s while debugging a problem with conditional_fields setting #required state, I found a couple issues with this patch in dependentValidate() where:

1. paragraphs within paragraphs weren't being treated quite right: array_search() was finding the first subform (top-level) instead of the one directly containing the field in question
2. $field_name was often not being computed correctly because it was using ['field_parents'] instead of ['array_parents'].

The only effect of these issues might be that certain validation messages don't show the correct name of the field... but it does appear to be a logical bug, and made debugging more complicated.

@scott_euser I don't mean to be a jerk about not including the other issue patch, but this issue is already really tangled.

Re-rolled for latest 4.x-dev.
(A couple line changes no longer necessary because fixed in 🐛 Warning: Undefined variable $entity_type in Drupal\conditional_fields\Form\ConditionalFieldForm->buildTable() (line 297 Fixed (commit).

Re: #134 it'd be better to keep this patch constrained to what this issue is about. If people need multiple patches they can apply and resolve potential conflicts themselves. Otherwise the issue gets convoluted and harder to evaluate/review/commit. At the extreme logical extension, every issue patch contains every other issue patch.

Missed an isset().

< +        if ($input[$param]) {
---
> +        if (isset($input[$param])) {

jeffschuler → created an issue.

hook_entity_bundle_field_info_alter() says it will be deprecated:

@todo WARNING: This hook will be changed in  https://www.drupal.org/node/2346347 → .

Is that still the hook these docs should suggest using?

jeffschuler → created an issue.

Great. Thanks @paper boy → !

Either explain or remove the "WetPersonallisten" that prefixes the Constraint ID ("WetPersonallistenStartEndTimeConstraint"), please.

Fixed how the filter options list is saved – as a sequence of strings instead of bools.

Including the interdiff from #3 and another combined patch with 🐛 Fix base_url issue Closed: outdated #7 that you can probably ignore (see my comment in #4).

Here's a combined patch of both the fix in 🐛 Fix base_url issue Closed: outdated #7 and the patch here in #3, as the two don't merge together cleanly and I'm currently running both on a site.

Not for testing or evaluation unless you're in a situation like mine.

Missed conditional for whether the attribute value was true.

Note that if we do copy core for this, there's a bit of a mistake with where Html::transformRootRelativeUrlsToAbsolute() puts its srcset logic – it shouldn't be within the loop over attributes.

I opened an issue for that: 🐛 Logic error in Html::transformRootRelativeUrlsToAbsolute() Fixed .

jeffschuler → created an issue.

jeffschuler → created an issue.

@nareshbw: Can you include more detail on steps to reproduce, expected results, and actual results?

Could it be that rel_to_abs' input filter ("Convert relative paths to absolute URLs") needs to come after any filters that ckeditor_uploadimage provides?

@dionhalcyon can you describe your last step ("Created a views rest in json format") in a little more detail?

I just created a View with a the core "REST export" display, outputting content with a body field that uses the rel_to_abs input filter, and it works for me – with the "Show" setting (in the REST export Views display Format settings) set to Fields as well as to Entity.

Reroll for latest dev. No other changes.
Thanks @podarok for adding 2.2.x-dev!

@Raveen: When you comment on the issue, you can change the Status to RTBC. I am doing so.

This was already fixed in #2079001: Network path reference urls (//) should not be prefixed → .

There's a check:
&& substr($txt, $pos, 2) != '//' to ensure the string doesn't start with "//".

The attached patch adds a simple case to the module's functional test to verify this.

This module only operates on text that's output using Drupal's Text formats, in which you can enable the input filter that the module provides. Usually this is for field data. Maybe you can figure out how to produce this text by some other means (like field-based output) where you can use the input filter.

Have you enabled the "Convert relative paths to absolute URLs" filter in each of your Text formats (https://example.com/admin/config/content/formats) ?

srcset can include multiple URIs.
See https://html.spec.whatwg.org/multipage/images.html#srcset-attribute

We should probably do as is done in Html::transformRootRelativeUrlsToAbsolute(): explode the comma-separated string that's within srcset="" and operate on each URI within.

Right now this module doesn't track where the attribute value ends, though. It should probably also mimic transformRootRelativeUrlsToAbsolute() and use XPath. But maybe you can keep this change simpler for the moment and look for the end quote of the attribute string?

jeffschuler → created an issue.

Since getSchemeAndHttpHost() just returns scheme & host (and not another potential part of the path, like <front> might) we don't need to mitigate that, so I've removed that logic.

I've also modified the functional test to use the same method for getting $base_url, and checked that the test passes.

Changing version here to 2.2.1, though it would be nice to have a 2.x-dev choice in the Version choices.

+1 RTBC

Yes, let's get this in there.

Re-rolled for 2.2.1 / 9.x-1.x.