Account created on 11 October 2006, over 17 years ago
#

Recent comments

🇺🇸United States jenlampton

(adding code tags)

🇺🇸United States jenlampton

This looks like a good safety check. marking as RTBC.

🇺🇸United States jenlampton

I agree, this is a nice improvement. Marking as RTBC.

🇺🇸United States jenlampton

Patch nolonger applied to 7.100. rerolled.

🇺🇸United States jenlampton

Thanks @douggreen!

🇺🇸United States jenlampton

> #51 suggested linking to WCAG somewhere, but that was never done.

We could link to https://www.w3.org/TR/2012/NOTE-WCAG20-TECHS-20120103/G201.html

The text they include in that example is (opens in new window) so I'm tempted to always use that exact text and not allow for it to be modified. The only use-case for modification would be if someone had already entered the missing warning into the existing link is external text. Making neither configurable would remove that use-case, but I also dislike the idea of removing options that other people think are helpful, especially when they are already out there in use on production sites.

🇺🇸United States jenlampton

Accessibility isn't optional. Why is there an option for "Add "(New window)" in the title of links that should open in new window."? The answer to that checkbox needs to be identical to the answer to "Open external links in a new window or tab." So we don't need both.

Either the links are opened in new windows (AND they have a warning) or they are not (and there is no warning). There is not a use-case for having links open in new windows without a warning. That's not accessible.

🇺🇸United States jenlampton

Updating issue description to be more comprehensive.

🇺🇸United States jenlampton

It looks like people are using https://www.drupal.org/node/3054538 to discuss the warning, so I'm going to close this issue in favor of that one.

🇺🇸United States jenlampton

I'm removing the installer module screenshot from the issue description since installer module in Backdrop, and Backdrop upgrades from Drupal 7 have nothing to do with each other. I know some people like to do upgrades that way but since that isn't supported (and can be dangerous) I don't want to encourage it ;)

We do still need the complete list of module machine names for supported upgrade paths (and people can use that list for whatever they want!)

Please also see the GitHub issue for more discussion and other possible solutions:
https://github.com/backdrop-contrib/backdrop_upgrade_status/issues/44

🇺🇸United States jenlampton

I don't think this information belongs on the "Backdrop Upgrade Status Report" page. It doesn't have anything to do with how ready your site is for Backdrop (and the report page is already very long).

The complete list of modules could go on the Drupal Status Report page, maybe? We could add it there.

We could also add it to the top or bottom of the "Backdrop Upgrade status" page. That page already lists all the contrib modules so it seems like maybe a good fit.

I was also thinking that we could add a page for "Backdrop Upgrade Tools" (or something). A drush or bee command could go there. But neither would really be necessary if we had the complete list of modules, so maybe that's all we need to add.

🇺🇸United States jenlampton

See related: https://www.drupal.org/project/extlink/issues/3054538#comment-15555115 A11y: add screen reader text for external links Needs review

🇺🇸United States jenlampton

> Since the script checks (I assume) these locations anyway, would it be possible to add links to each of the contrib versions it finds into the report to make it super easy to find them?

This is a great idea. I've gone ahead and added the name of the project -- linked to its repository -- to the backdrop upgrade stats report in b1d6c8af7d59b253d48b18b3d93d4d0c57e89bcb.

Leaving this issue open so we can look into the OpenAtrium issues.

🇺🇸United States jenlampton

I've updated shortcut to indicate the Backdrop contrib version.

I do not know why Search Krumo and Invite were not identified as contrib modules in the screenshot. In my testing they were properly identified and a recommended version was displayed. I suspect it may have been fixed since it was reported.

🇺🇸United States jenlampton

Added a link that says "See the documentation on supplemental contrib modules"

🇺🇸United States jenlampton

I'm not seeing this problem in the latest release, it looks like it was fixed with 3e59634d40e6.

🇺🇸United States jenlampton

> After installing Backdrop Upgrade Status, all Paragraphs edit fields started displaying the Main menu instead of the content of their paragraph.

That's really bizarre! I've tested both and was not able to reproduce this. I might need a little more information. What kind of fields were on your paragraphs? Were they standard text / long text or anything specific? Was it all the fields on a paragraph, or only some of them?

🇺🇸United States jenlampton

Great point - I've added the text "No contributed modules detected. Run cron or check manually." (with links to each of those two tasks) as the "empty" value for the list of contributed modules.

🇺🇸United States jenlampton

As far as I can tell, Date repeat doesn't appear to be deprecated, or at least, I could't find that reference. I checked several versions of the README: https://github.com/backdrop-contrib/date_repeat. Could you drop in a link to where you saw that documented?

Repeating Dates is currently listed as a similar module, but not a replacement. I don't think we should direct people to use a similar module without a supported upgrade path as part of the upgrade, unless Date repeat is definitely deprecated.

🇺🇸United States jenlampton

Since merge isn't working on gitlab, I pulled a patch and applied it manually instead.

🇺🇸United States jenlampton

Changing to RTBC after the last comment.

🇺🇸United States jenlampton

That issue appears unrelated to our problems with files. We're now working over here: https://www.drupal.org/project/drupal/issues/3424701 🐛 Cannot delete file when using language negotiation domains Active

🇺🇸United States jenlampton

This issue summary recommends a fall-back to the edit form, but there are entities that don't have edit forms (files, for example). We have a site with translation that's unable to delete files with this patch. We can also reproduce the problem reliably on other sites if we attempt to delete a file without any destination query argument.

Does anyone know if there is another follow-up issue to address that? I searched but couldn't find one.

🇺🇸United States jenlampton

We are also using this patch on several projects. Marking as RTBC.

🇺🇸United States jenlampton

Could we ask that the documentation be updated for Drupal 10? We're not able to get the module to work at all, and the settings seem very different from the code snippet we are provided by the ShareThis website.

We're currently evaluating possible services to use now that AddThis has died, but ShareThis doesn't seem to be very alive for Drupal either :) Some documentation on how to set it up would be very helpful.

🇺🇸United States jenlampton

The menu item is still where it was under Structure, but the URL has changed to match admin/content. I think the update hook might have been overlooked for existing sites?

🇺🇸United States jenlampton

> First of all, can you let me know the machine name of the "Type 1: Edit content block" permissions

The machine names for the permissions are as follows:

* Type 1: "Basic block: Edit content block" is edit any basic block content
* Type 2: "Card: Edit content block" is edit any card block content
* Type 3: "Promo Box: Edit content block" is edit any promo_box block content

> this pattern doesn't seem to match existing or new core permissions for editing block content entities:

That's strange, I copy/pasted these from the permissions page. Maybe it's different in D11 than in D10?

> Do you have other modules providing these permissions? Perhaps https://drupal.org/project/block_content_permissions?

Nope :)

> Looking at the access handler changes to the update operation from the above issue, you also need access block library to edit blocks.

This is my problem, I think. That completely defeats the purpose of the block-type edit access, doesn't it? If you don't want anyone to know about any blocks other than the type they can edit, why would you want them in the library?

Do people need access to the admin/content page in order to edit a single node type now too? If so maybe I missed that change...

> This permission was added in an upgrade path, but only for roles with administer blocks - perhaps it should have added it to roles with the "edit any" permission too?

No no no -- we definitely wouldn't want to grant people more permissions than they had before. That sounds like a potential security issue, no? We do, however, want them to have *the same* level of access as they had before.

> EDIT: Had a quick chat internally larowlan and the other option would be to remove the access block library check for these operations - there was a reason it was included but we need to do some issue forensics to figure out why!

I think this is the best option. I'll see if we can start working on that.

🇺🇸United States jenlampton

Block permissions that were working in D9 don't seem to be working anymore in D10.

In drupal 9 our editors had the following content block permissions:

Type 1: Edit content block
Type 2: Edit content block
Type 3: Edit content block

With these permissions we were able to build an admin listing of content blocks (using views) that contained the "Bulk operations" field. Editors could use the edit link here to edit these Blocks. Important note: This view's Access is set to Permission: Type 1: Edit content block

After upgrading to Drupal 10, editors can still access this view (so the permission works in general), but all the edit links are missing from the "Bulk operations" column (so the permissions do not work for editing blocks anymore)

For developers (admin role) both edit and delete links are present under "Operations".

What I tried:
* Does not work: Adding the views field for "Link to edit Content block" -- this link appears for developers (admin role) but not for editors.
* Does not work: Adding the views field for "Block content ID" and rewriting this field as a link /admin/content/block/{{ id }} -- this link appears for editors, but when attempting to edit the block they still get "Access denied"
* Does work (with consequences): Granting editors the Administer block content permission -- this restores the edit links for editors, but it also adds a delete link, and grants them edit/delete access to additional block types that they should not be able to access.

🇺🇸United States jenlampton

Thanks so much! I've also added Juan Olalla who is one of my teammates.

🇺🇸United States jenlampton

This one is created from the correct directory, and includes the new "View" default local tast.

🇺🇸United States jenlampton

Oops, I forgot an important function :) updated patch.

🇺🇸United States jenlampton

I have added this functionality for a project, contributing back now.

🇺🇸United States jenlampton

You're right we added that. So sorry - I thought it was being added by webform!
Closing (and sorry for the noise)

🇺🇸United States jenlampton

Additionally, when we attempt to remove the for attribute using Twig in the template for the webform element form-element--webform-processed-text.html.twig, the entire label disappears.

  {{ label.removeAttribute('for') }}
🇺🇸United States jenlampton

Clarified the upgrade path section. Added the Drupal 7 -> Backdrop process and elaborated on the Backdrop -> modern Drupal process.

🇺🇸United States jenlampton

I've merged the merge request, this change will be included in the next release.

🇺🇸United States jenlampton

I'm seeing this same notice for numeric handler:

Undefined index: value in views_handler_filter_numeric->accept_exposed_input() (line 399 of www/sites/all/modules/contrib/views/handlers/views_handler_filter_numeric.inc).
🇺🇸United States jenlampton

> I don't think we should add this warning text though "'Warning: people may not see the updated content of static files (.txt, .pdf, etc) because they can be cached by external caches for a long time." - we cannot know how people have configured the caching of their static assets on their sites. This could lead to more confusion.

This warning is critical for our editors, specifically because external caches often tend to ignore any static asset caching rules as defined by the origin server. The static asset caching on the origin server is also not something people usually have access to (not through Drupal, anyway) and by default it is set to "forever".

This means that 99% of the time, using the same file name for a PDF will result in people not seeing updates to that file -- which is why Drupal adds the _0 and _1 to the end of the files in the first place. It's true that we can't know if people are in the 1% who have mitigated these risks, but I'd prefer that we make life less frustrating for the 99%.

Drupal tries to save people from this confusion by not explaining anything, and automatically adding the _0 and _1, which often leads to frustration for editors. Here we're giving them the choice to avoid that frustration, by potentially causing another.

I would like to explain the risks associated with using the same file name, so that if people do choose to keep it, they won't be frustrated by not seeing any changes to the file they keep replacing.

I agree that this explanation may lead to confusion, but that's mostly because most editors don't already understand that static asset caching is forever, and that things like PDFs are considered static. I'd prefer if we could iterate until we find something better, rather than removing the warning entirely. :) Do you have recommendations on how to improve the language to make it less confusing?

🇺🇸United States jenlampton

Just adding another reminder here that applying the latest patch would also be introducing a security risk for your site. Use with caution.

🇺🇸United States jenlampton

Whoops, forgot the api file. Uploading again :)

🇺🇸United States jenlampton

Rerolled this patch against the module location. Otherwise this is RTBC from me.

🇺🇸United States jenlampton

Posting a revised patch that also converts the template file into a theme function.

🇺🇸United States jenlampton

> These offers needs to be posted on the project issue queue. They are moved on the Drupal.org project ownership queue after 14 days, if the project owner or one of the maintainer has not replied.

@apaderno There is no owner or maintainer for this project, as stated in the original issue. What is the point of waiting 14 days for nobody to respond?

> File an issue in the queue with a patch to fix the module and then contact the security team to have your version reviewed and the project handed over to you

I have done all of this already. I don't know what the next steps are if this issue is not what I'm supposed to do.

🇺🇸United States jenlampton

Here is my first pass at a patch that adds some basic sanitization in places where it seemed problematic, but I'll need to more than basic testing to ensure everything still functions as expected:

https://www.drupal.org/project/link_click_count/issues/3353002 📌 Security Hardening + English Clean-Up Active

Production build 0.69.0 2024