I don't even consider the proposed non-group solution as an actual one. I'm just imaging a point where someone from the DA / infra team (or any other consumer of the module) needs to temporarily revoke a permission for some reason. That would require a patch!
Are there any issues with my proposed group solution? I'm willing to address these.
What is the state of Update groups to 3.x? I did not check if its still applies there. If not i guess its a blocker for this issue.
In order to perform the access check on the group entity on the REST endpoint the L10nServerContributorResource::access() is probably not needed. But query access check has to be turned on.
The recently added submit suggestions remotely permission could be checked on the route.
A correction on the first case, the custom access handler. It wouldn't be enough to check if the user is just a member of the group, because then they would have the same permission on every group. Instead they would need a specific role within that group. The permission itself could be defined / resolved in code.
I'm not sure how to solve this.
The issue occurs because the access handler grants access when either group or account permission have been granted and authenticated users have the required core permission as mentioned above.
The most straightforward solution would (probably) be to just re-use the (existing) core permissions and change (the above mentioned) access handler resolve the group permission itself. I.e. by checking if the user is a member of the group and has the required core permission. But the group ownership of a translation string is not resolvable from a translation entity as they only have a (two-letter) abbreviation of the language and group entities indicate their language only by their label. Therefore we would need to resolve that mapping issue beforehand.
The alternative path would be native group permission. They are bound to group relationships. For our stories and wikis these are provided by group's node relationship plugin. A custom one for our l10n_server_translation entity type would be needed. That's part one. The second part are the relationships. There are usually created during entity creation. That would potentially require changes to migration. To prevent those additional relationships we could override the storage handler of the group_content entity type. In that case there the need for a custom translation to group entity mapping too.
Maybe that same principle applies to the permissions and actions of Localization Server + Localization Packager.
Maybe there is another (more straightforward) solution, but until now i haven't find one.
Unfortunately the export of a project, that consists of a large code base, like core, has major issues. The generation takes quiet some time and the produced
translation files are very large when the Include metadata (Verbose output on 7.x-1.x)) option is enabled. They have a header that includes a Generated from files listing. These files are prefixed with the version string when releases are merged.
On my local i've run several exports of the German translation of Core where almost all releases, that are listed on the core's update endpoint, have been parsed. That's ~630 releases, starting from 4.5.0.
The results are:
- All flags enabled (Download untranslated and translated strings + Inject German suggestions? + Include metadata):
The generation took 36min and produced a 337MB file.
The above mentioned file list is 724084 lines long where most of them look like this: # drupal-10.3.x-dev/core/themes/starterkit_theme/starterkit_theme.info.yml: n/a. I've seen only a few that look differently like that one: install.inc,v 1.24 2006/10/23 06:45:17 dries
Every msgid / msgstr item has a reference to a source file and line number. When releases are merged there is a reference to for every release. That makes them really long (multiple thousand characters). E.g. #: core/authorize.php:146; core/lib/Drupal/Core/Updater/Module.php:130; core/lib/Drupal/Core/Updater/Theme.php:110; ...
Just in case someone is interested, i've attached the compressed file.
- When the Include metadata option was unchecked (and Download untranslated and translated strings + Inject German suggestions? where checked) the generation took 24.69min and produced a 3.9MB file.
- The fastest export took 11.82min and produced a 311.8MB file (with Download only translated strings + Include metadata checked and unchecked Inject German suggestions?) / 13.8min with checked Inject German suggestions? and produced a 315.5MB file
---
That's on modern hardware (Zen 3 CPU + NVMe), running on Linux.
I've never used a local translation application but can hardly imagine that this amount of data is actually useful. I wonder what the value of having these file listing is!? Can those large files even processed by local translation applications? Are they able to handle / visualize that many references? Do they remove the metadata? If not the files are too large for an import (file size limit is 50MB on production).
The batch rewrite is still (probably) required, because the timeout happens when a single release is exported too, but i think we should limit the All releases merged option to projects that are below a specific threshold (e.g. a line count and / or release count).
Was able to reproduce it on current 3.0.x-dev and i'm working on a fix.
shmy → created an issue.
By the way the in the latest commit of the MR 46 i've reverted #3396437 partly but omitted the revert for the submissions.js file because its part of l10n_statistics submodule. Not sure how to handle this (re-opening the issue? new issue?).
:face_palm: While working with this old code base and seeing so much copied 7.x it didn't crossed my find that there could a form validation implementation and instead i did a quick grep for any 'error' string.
Anyway i've addressed the above mentioned issues.
shmy → changed the visibility of the branch 3.0.x to hidden.
While working on #3320795 i've checked this and the author link and both throw an exception. Just for context purposes:
Related projects (path e.g. /translate/source-details/1):
TypeError: Symfony\Component\HttpFoundation\Response::__construct(): Argument #1 ($content) must be of type ?string, Drupal\Core\Render\Markup given, called in /var/www/html/web/modules/contrib/l10n_server/l10n_community/src/Controller/L10nCommunityLanguagesController.php on line 618 in Symfony\Component\HttpFoundation\Response->__construct() (line 224 of /var/www/html/vendor/symfony/http-foundation/Response.php).
Author detail listing (path e.g. /translate/translation-details/4)
TypeError: Symfony\Component\HttpFoundation\Response::__construct(): Argument #1 ($content) must be of type ?string, Drupal\Core\Render\Markup given, called in /var/www/html/web/modules/contrib/l10n_server/l10n_community/src/Controller/L10nCommunityLanguagesController.php on line 577 in Symfony\Component\HttpFoundation\Response->__construct() (line 224 of /var/www/html/vendor/symfony/http-foundation/Response.php).
Alignment should be ok now:
How do i reproduce the .error class as mentioned in #1? I've seen how it looks on D7 (when adding the class manually) but would be nice too know how / when its set.
Thanks for taking care of BC.
I think the required change depends on issue classification.
In my eyes this is a bug fix, not a feature request, because as a user (i.e. site builder) i don't expect that my hard-coded markup is filtered out when summarized.
Since the activity (and probably demand) for a fix seems not very high why not make it 11.x only?
I confirm that the UI / UX differs as described in #10 + #12 + #13. Already found and fixed the #13 issue locally. Will continue soon'ish.
As the implemented solution has been approved during review, i belief the proposed solution and remaining task can be updated in the issue summary. Back to review.
As the ensureMarkupIsSafe renderer method the treats MarkupInterface as safe and TextTrimmedFormatter::preRenderSummary is a public static method and therefore may be called within a different context by contrib/custom modules (e.g. a render element that doesn't preserve the #pre_render callback preRenderText) iv'e tried to find a more generic implementation but as MarkupInterface doesn't dictate the `::create` method i couldn't find a practical approach.
As test coverage is needed anyway i hope to have some time in the next days and extend an existing ones or add a new one if necessary.
I've extended the patch ( interdiff_9_19 → ) by a safety check to ensure only markup, that is already passed as filtered, is back-casted from string. I'm not sure if the taken approach is sufficient.
Here are screenshots for renamed sort--inactive.svg image tables.pcss.css file:
Before (default):
After (default):
Before (forced-colors):
After (forced-colors):
The :is() selector doesn't work for me in neither Firefox or Chromium. I haven't worked with PostCSS before and i don't get why its added there. The following screenshots are made w/o the :is() selector as there is no difference for me when set. I don't know if this is a different issue or i'm making something wrong.
Before (RTL default):
After (RTL default):
Before (RTL forced-colors):
After (RTL forced-colors):
shmy → changed the visibility of the branch 3332701-refactor-claros-tablesort-indicator to hidden.
shmy → changed the visibility of the branch rebase-testing to hidden.
I've converted the #35 patch to a MR.
As the rebase on 3332701-refactor-claros-tablesort-indicator is broken i did a new one on rebase-testing (sorry for the poor name). Someone who actually worked on the code should review the result. I run the build:css script after every during every merge conflict and after the final one a lint:css
shmy → made their first commit to this issue’s fork.
shmy → made their first commit to this issue’s fork.
I've compiled the committed CSS and didn't got any modifications on the CSS file too, which makes sense as this is just different nesting.