San Francisco
Account created on 29 September 2004, over 19 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States mfb San Francisco

Closing for now but feel free to re-open if anyone wants to get spotlight working

🇺🇸United States mfb San Francisco

@dsnopek I would suggest backporting the actual accepted patch, assuming you find that it works on Drupal 7

🇺🇸United States mfb San Francisco

@Anybody ok so you're using ubuntu /etc/mime.types, not apache. In that case, looks like webp was added more recently, in January 2023, i.e. in Ubuntu 23.04 - https://salsa.debian.org/debian/media-types/-/commit/6883baf645560bcfe0a... - so it's finally fixed there too

🇺🇸United States mfb San Francisco

FWIW, I checked the apache httpd source to make sure they do have webp in the default mime.types file, and it was added 13 years ago... https://github.com/apache/httpd/commit/217485c7e1f2f499049b4ff745076e4ed...

🇺🇸United States mfb San Francisco

mfb created an issue.

🇺🇸United States mfb San Francisco

So the URL filter, which can be found in the _filter_url() function in filter.module, was developed in the context of previous versions of Drupal, where HTML was serialized by calling saveXML(), thus outputting literal UTF-8 characters, since XML doesn't support most HTML entities. After HTML 5 support was added, HTML serialization results in HTML entities rather than literal UTF-8 characters.

I think the URL filter could be overhauled to support this use case? Parsing HTML with regex is generally frowned upon, but that's what this filter does. If HTML was parsed properly then text nodes could be modified, regardless of HTML entity encoding? Tweaking the serialization rules might be possible too, to partially restore previous behavior, but that seems really hacky.

🇺🇸United States mfb San Francisco

maybe a comment about how this open-ended

Can you/someone confirm that options are intended to be open-ended (e.g. for contrib module to add/use arbitrary options)? If so, yes, that would be good to document

🇺🇸United States mfb San Francisco

$_ENV should be fine to use with those dotenv libraries. But, if you are setting environment variables the old-fashioned way, and using the so-called "recommended" PHP configs for development or production, $_ENV will be empty and all the environment variables will be in $_SERVER

tldr: PHP is fun

🇺🇸United States mfb San Francisco

@mstrelan the issue is that when setting environment variables, e.g. from an .env file, the app may only set them into $_SERVER and $_ENV, not by calling putenv(), so those environment variables will not be available when calling getenv()

🇺🇸United States mfb San Francisco

@mglaman there's also a problem with $_ENV, which is that the recommended php configs don't enable it (the recommended value for variables order is "GPCS" rather than "EGPCS", i.e. $_ENV is not registered). So, at least in my experience, $_SERVER ends up being the most universal place to read environment variables from.

🇺🇸United States mfb San Francisco

The tldr explanation is that if PHP is thread-safe, and you call putenv() to set environment variables from a .env file, the environment variables will be set on the whole process, including other threads where they shouldn't be. This problem shouldn't affect server environments where each request has its own process, but in any case, symfony dotenv disables the usePutenv option by default, and to use putenv() with phpdotenv, you have to call Dotenv::createUnsafeImmutable() rather than the usual Dotenv::createImmutable().

🇺🇸United States mfb San Francisco

I would say environment variables should be read from $_SERVER rather than getenv(), as putenv()/getenv() are generally discouraged due to not being thread-safe

🇺🇸United States mfb San Francisco

@quietone did you mean this is a duplicate of Make the map of PSR3 to RFC 5424 log level constants available as a constant Needs work ? I don't think it is a duplicate, as far as I can tell, speaking as someone who has commented on both issues and created one of them. So, if folks are interested in migrating to the PSR-3 log level constants they should probably re-open this issue.

🇺🇸United States mfb San Francisco

@quietone I actually suggested closing this issue, as it was pretty trivial to copy the map into two contrib modules where this feature could've been useful. Btw, I don't see how that other issue is a duplicate?

🇺🇸United States mfb San Francisco

Updated the issue summary to explain why this internal method data may be useful to other loggers, not just Drupal core.

🇺🇸United States mfb San Francisco

@cmlara the PSR-3 logging standard provides a context array to pass arbitrary context data to loggers. So that's what I'm proposing to use (it may be helpful to somehow namespace usage of the context array, however, as I mentioned in the issue summary)

In Raven module we try to figure out if an error/exception was more-or-less "handled" somehow (i.e. caught, logged and ignored), or more-or-less "unhandled" and logged by one of the final error/exception logging mechanisms.

At the moment, core doesn't add anything to the context array about the disposition of the error or the mechanism logging it, so the best a logger can do is try to figure that out by looking at the backtrace. Which works fine, but is a bit hacky for my taste.

🇺🇸United States mfb San Francisco

This needs work, as a hook allows other modules to add other form IDs, for which the schema is missing/invalid.

🇺🇸United States mfb San Francisco

@smustgrave well, as I mentioned in remaining tasks, the $options might in fact be designed to be open-ended? In which case this could just be documented as mixed[]. (If so, should be needs work for that)

🇺🇸United States mfb San Francisco

@pcambra hopefully this resolves the issue, but please re-open if not

🇺🇸United States mfb San Francisco

That's great, when it's released can you ping this issue and I'll add a link on the project page

🇺🇸United States mfb San Francisco

Moving feature request to 6.x branch; perhaps I should also close this as I'm not actively working on it

🇺🇸United States mfb San Francisco

Moving feature request to 6.x branch

🇺🇸United States mfb San Francisco

Moving feature request to 6.x branch

🇺🇸United States mfb San Francisco

I doubt anyone will care specifically about the message variable when looking at a stacktrace, but we could create a new variable for the modified message for "correctness". Can be done on 6.x branch and backported.

🇺🇸United States mfb San Francisco

mfb created an issue.

🇺🇸United States mfb San Francisco

Probably can postpone this until the Drupal 11 beta

🇺🇸United States mfb San Francisco

mfb created an issue.

🇺🇸United States mfb San Francisco

re: compatible with HTTP/2, I'd hope that proxies convert HTTP/1.1 chunked transfer to HTTP/2 (or HTTP/3) data frames and pass them along, without excessive buffering, but not something I've analyzed

🇺🇸United States mfb San Francisco

As far as I've seen, Big Pipe does send chunked? All you have to do is print something and then call flush(); and the Transfer-Encoding: chunked header should be added automatically. HTTP/2 or HTTP/3 don't support it though, so you might not literally see a chunked transfer, depending on the stack.

🇺🇸United States mfb San Francisco

mfb created an issue.

🇺🇸United States mfb San Francisco

Sounds like an excellent improvement

🇺🇸United States mfb San Francisco

mfb created an issue.

🇺🇸United States mfb San Francisco

mfb changed the visibility of the branch 3408332-backport-4.x to active.

🇺🇸United States mfb San Francisco

PHP is designed to be configurable at compile time, so patches aren't necessarily needed. You can build with or without various extensions/libraries, whether for running in a memory-constrained environment or any other reason. On the one hand, I'd appreciate if PHP could be guaranteed to be built the same way, but on the other hand I can appreciate it being configurable..

🇺🇸United States mfb San Francisco

@andypost It's not true that the zlib extension is always bundled into PHP. On FreeBSD, for example, it is a separate package which is installed via pkg install php83-zlib and I'm sure some other distributions work in a similar way. I.e. each distro has its own definition of the PHP package and provides packages for the other extensions.

🇺🇸United States mfb San Francisco

Since the call to drupal_wrap_mail() is removed, the comment cannot says and wrap the mail body for sending, as that is done by drupal_wrap_mail().

drupal_html_to_text() wraps the mail for sending. As the function documentation states: "The output will be suitable for use as 'format=flowed; delsp=yes' text (RFC 3676) and can be passed directly to drupal_mail() for sending." Within drupal_html_to_text(), drupal_wrap_mail() will be called on each chunk.

The changed line uses " \n" (two spaces and a new line) instead of " \n" (a space and a new line), but the other call to wordwrap() uses " \n" (a space and a new line). I am not sure this is a typo or a inconsistent change.

The trailing space will be deleted when format=flowed "delsp" email is formatted for display. When wrapping at a space, you want to preserve the space, so you end up with two spaces at the end of the line. For extremely long words, there should be no space when the word is reassembled for display, so you have just one space at the end of the line.

With this change, I am simply reverting 273c78ded2005cac80e2fa965f9131a85be16f0b, so this code goes back to it's original logic in #154218: Add HTML to text convertor for email (and wrap mails properly) (mostly, there was apparently an additional bug fix in there meanwhile)

If this seems too unclear, I could add additional verbiage to the code comments?

🇺🇸United States mfb San Francisco

I would suggest adding zlib to system_requirements() as well, because of various scenarios such as zlib extension could be disabled after the instance was installed, instance could be moved to an environment without zlib extension, etc.

🇺🇸United States mfb San Francisco

@catch thanks for further explanation. It surprised me to see how many cache_data insert queries run on a cold cache: a "js:..." cache item is inserted during the page load, and further "js:..." and "route:..." cache items are inserted during asset load/generation. So that's why I was thinking the data in the URL could simply be persistently stored and queried rather than passed around. But yes, those cache sets would be a lot more lightweight with a non-database cache backend in place.

🇺🇸United States mfb San Francisco

Change record ready for review.

🇺🇸United States mfb San Francisco

This is I think ready to be committed to 5.x branch, and then we can backport to 4.x as well

🇺🇸United States mfb San Francisco

Ok, but that might essentially be Linux if docker-based? Would be excellent if someone could test directly on raw macOS

🇺🇸United States mfb San Francisco

@smustgrave were you able to run the newly-added test on macOS - that would be the main thing worth testing.]

🇺🇸United States mfb San Francisco

mfb created an issue.

🇺🇸United States mfb San Francisco

mfb changed the visibility of the branch 6.x to hidden.

🇺🇸United States mfb San Francisco

Let's add this feature to the 6.x branch

🇺🇸United States mfb San Francisco

Well, in the contrib module that this would've been useful for, I ended up introducing my own LogLevel enum to manage all the log level logic :)

So, I don't really need this anymore; feel free to close this issue as far as I'm concerned (or, perhaps the original feature request could still be useful for other developers).

🇺🇸United States mfb San Francisco

...well what I ended up doing, to allow submitting empty string to a nullable float, was use a ConfigTarget callback to convert empty string to NULL (passing through any other string), and that seems to work fine.

🇺🇸United States mfb San Francisco

I'm also hitting the "This value should be of the correct primitive type" error with float configs, when I refactor the form to use the shiny new-ish #config_target functionality, and submit the form with blank/empty number form fields, which previously worked fine for submitting NULL values.

I did add nullable: true to the config schema, but still get the same error when I submit the config form. I guess either PrimitiveTypeConstraintValidator should be made aware that the value is nullable and therefore empty string should be cast to NULL, or I need a way to disable/override this validator?

🇺🇸United States mfb San Francisco

mfb changed the visibility of the branch 5.x to hidden.

🇺🇸United States mfb San Francisco

mfb changed the visibility of the branch 4.x to hidden.

Production build 0.67.2 2024