Follow-up to #1804688: Download and import interface translations. See comment #99 also.
Problem/Motivation
admin/config/regional/translate/import
has help text For situations where translations need to be manually imported, this page imports the translated strings contained in a Gettext Portable Object (.po) file, which can be downloaded from the Drupal translation server.
and in #1804702-81: Display interface translation status @Sutharsan mentions
Re. rewording: I have avoided using "the community translation server". l.d.o is the default translation server but can be replaced for example in case of distributions which have their own server. And l.d.o calls itself "Drupal Translations" Perhaps better to call it "Drupal translations website" instead of "... server".
There are other pages that also use "Drupal translation server" as well.
We should focus on UI strings and not the use of "translation server" in the comments and code.
Proposed resolution
Use "Drupal translations website" instead of "Drupal translation server" in UI strings and text files (e.g. INSTALL.txt)
Remaining tasks
- (done) Write patch to make that change.
- (done) Review patch.
Files that need changing:
- For installation process:
- core/INSTALL.txt
- core/includes/install.core.inc (for installation process)
- Import UI translations:
- core/modules/locale/locale.module
- core/modules/locale/locale.install
User interface changes
Yes, changes to user interface text, this is a ui issue, see above.
Pages that have UI change:
- installation process
- admin/config/regional/translate/import
- admin/config/regional/translate/settings
API changes
No API changes anticipated.
| Comment | File | Size | Author |
|---|---|---|---|
| #61 | change-server-to-website-1861930-61.patch | 9.15 KB | nonsie |
| #56 | install-after.png | 10.19 KB | sun |
| #56 | install-before.png | 11.34 KB | sun |
| #56 | distro.translation-website.56.patch | 13.17 KB | sun |
| #53 | change-server-to-website-1861930-53.patch | 8.6 KB | yesct |
Issue fork drupal-1861930
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #1
yesct commentedIf people don't mind, this small change could be done and not be postponed. It makes a nice novice issue.
Comment #2
tstoecklerI'm wondering whether we shouldn't utilize drupal_install_profile_distribution_name() here for distribution-friendliness. I haven't checked recently whether that is actually the case, but in theory a distribution should be able to wipe the term "Drupal" completely from the user interface. I think the pattern that we usually use would be:
Of course not every distribution will create its own translation server, so this might be misleading as well...
Comment #3
yesct commentedI think in this case it really is the Drupal server. If it's another server they configure... it would have a different name.
I'm not sure at what point another translation server can be specified.
Had the same question as part of looking at the flow charts on #1848490: Import translations automatically during installation
Comment #4
tstoecklerWell, e.g. OpenAtrium has its own translation server, so they would definitely want that string to read "OpenAtrium translations website" or whatever. AFAIK Drupal Commons does not have such a thing, so they might want it to read "Drupal translations website" or not, I don't know.
Just to be clear, I personally think the only consistent way to do this is to use drupal_install_profile_distribution_name(). That way we keep to our contract that we give install profile developers: "Put something in 'distribution name' and that will replace the word 'Drupal' everywhere in the UI." But, as mentioned above there might be cases where install profiles actually do want this to say "Drupal". It's not optimal, but I guess in those cases resorting to string overrides or whatever should be OK.
Comment #5
yesct commentedCan we check if the translation server is not equal to the drupal one (that a distribution overrode it) and if so, then do the replace of Drupal?
What would that code look like to check if the server is overridden?
Comment #6
yesct commentedactive since #1804688: Download and import interface translations is in.
Comment #7
yesct commentedtrying to really make it active this time.
Comment #7.0
yesct commentedadded issue number for general help text issue.
Comment #7.1
kristen polUpdated issue summary.
Comment #7.2
kristen polUpdated issue summary.
Comment #7.3
kristen polUpdated issue summary.
Comment #8
kristen polI updated the issue summary with some info on files that need to be changing and what pages are affected.
Note: I am assuming that we don't want to change the *internal* (developer) use of the term in comments and code that is not shown to the person installing/configuring Drupal. If we need to change the use of the "translation server" term *everywhere* (including internally-consumed text) then this will be a bigger issue and might not be suitable for a novice.
Comment #8.0
kristen polUpdated issue summary.
Comment #8.1
kristen polUpdated issue summary.
Comment #9
kristen polI made a list of files that need changing. I'll make a patch.
Comment #10
kristen polLooking in the code for localize.drupal.org, I find that it is not even used directly so localize.drupal.org is only referenced from within the comments and documentation. The translation files are grabbed using the server_pattern setting:
It seems logical to me to add a 'server_name' to the config (defaulted to "Drupal translations website") and then use that in any t()/st() functions where the translation server is mention.
Thoughts?
Comment #11
attiks commented#10 Adding server_name sounds like a good idea.
Comment #12
kristen polI'm working on a patch. Thanks!
Comment #13
gábor hojtsyMaybe we want to make it "the translations website" then and use the pattern set in config to link, and *that* would be general enough on the UI and flexible enough in terms of the link. However the pattern/url looks like would not lead to anything useful in itself, ha!
Comment #14
kristen polYeah, the "server_pattern" goes off to ftp.drupal.org so that's not useful. What about making a "server_url" config setting and then using the generic "the translations website" text and linking to the "server_url"? As a side note, it is confusing that "server_pattern" is used for the config name... it is not obvious that it is for translations. Maybe I should open an issue?
Comment #15
gábor hojtsyI think we used to want to have the human facing server URL as well, but then cut it back to only the necessity items. So that is how we ended up with only the download URL and not the nicely formatted real website URL.
Comment #16
kristen polSo, then, I shouldn't add it because it won't be accepted?
Comment #17
yesct commentedI'm confused..
Kristen, maybe work up a version in a patch so I can see better what is being discussed?
Comment #18
kristen polI'm waiting until:
#1861934: Rework help text for UI translations and importing po files
lands because there is some overlap in the text.
Comment #19
kristen polUnassigning from me in case the other issue gets resolved quickly and someone wants to do this during the sprint.
Comment #20
kristen pol#1861934: Rework help text for UI translations and importing po files was just committed.... adding sprint tag in case someone wants to work on it :) Otherwise, I'll try to look at it again in a couple days.
Comment #20.0
kristen polUpdated issue summary.
Comment #21
gábor hojtsySounds like this should not be postponed anymore. Keeping on sprint as a simple task for someone (for now) :)
Comment #22
manningpete commentedWorking on a patch on this today.
Comment #23
manningpete commentedThe files which needed to be edited are different than those stated in the issue summary due to other changes that have been made in the interim.
The two files which contained non-genericized references to the Drupal translation server, and which I edited were :
/core/modules/locale/locale.module
/core/modules/locale/lib/Drupal/locale/Form/LocaleSettingsForm.php
None of these files currently contain the "Drupal translation server" language, so I did not make any changes to them:
/core/INSTALL.txt
/core/includes/install.core.inc
/core/modules/locale/locale.install
I verified that the correct language "Drupal translation website" occurs during installation and on the following two pages:
/admin/config/regional/translate/import
/admin/config/regional/translate/settings
Comment #24
manningpete commentedNoticed the word "Translation" was capitalized on install page, so lowercased it for consistency.
Comment #25
gábor hojtsy@manningpete: thank you! There has been some discussion above from #2 as to whether it would be better to use the distribution name in place of 'Drupal' (which we do elsewhere). Can you look into updating the patch to have that suggestion incorporated? Basically using the pattern outlined in #2. Thanks again!
Comment #26
diego21 commentedThis is my first patch for D8. Hope it helps :D
Thanks!!
Comment #28
diego21 commentedMy first interdiff, although it seems the same that the patch file. I must have done something wrong.
Comment #29
diego21 commentedComment #30
mr.york commentedI didn't apply the last patch.
I created new patch.
Comment #32
gábor hojtsy30: change-server-to-website-1861930-30.patch queued for re-testing.
Comment #34
gábor hojtsyWhile fixing the bug, let's also get back to one of the original goals and replace "server" with "website" in the test too.
Comment #35
mr.york commentedAdd drupal_get_distribution_name() function in common.inc and replace "server" with "website".
Comment #36
mr.york commentedComment #37
nonsieHere are the screenshots of the modifications in place.
Installer screen:

Import screen:

Help screen:

Comment #38
nonsieShouldn't the copy be updated during install in requirements fail case as well?

Comment #39
yesct commentedYes I think we should change it in the requirements also.
Comment #40
nonsieAssigning to myself to work on incorporating #39 into this as well.
Comment #41
mr.york commentedReplaced "server" with "website" in all message.
Comment #43
gábor hojtsy41: change-server-to-website-1861930-41.patch queued for re-testing.
Comment #45
gábor hojtsy41: change-server-to-website-1861930-41.patch queued for re-testing.
Comment #47
yesct commentedtl;dr Thanks everyone! Keep on, we appreciate you.
-----
unassigning this.
It's not a huge deal, but usually, we use the assigned field to mean "I'm working on a patch". If it has been awhile since someone assigned it to themselves, sometimes another person will ask "SoAndSo, are you still working on this?" and then wait a day or two for a response before posting a patch. We do this because sometimes someone assigns it, but then doesn't work on it for whatever reason, like forgot, got busy, etc... in which case it is totally cool after a day to unassign it, or assign it to yourself and post a patch.
But... sometimes it might take the person it is assigned to several hours our days (because they are new, or because they are trying to do something new to them that is challenging), and we want to give them a chance to respond with something like: I'm almost done, patch coming soon.
It can be heart crushing to work for hours and hours on something new and difficult (for some individual) and to come back to an issue to post the result, only to find someone has already done quickly what they have just struggled to do.
The best outcome of that is that one of the persons can do a good review since they understand the problem. But sometimes people get discouraged and just give up contributing.
I hope *everyone* here in this issue knows that their efforts are *much* appreciated and continue to jump in when they feel they can help with something.
I'll look into putting some kind of instructions next to the assigned field so that this community knowledge is more exposed and people can know what it means when an issue is assigned.
Comment #48
gábor hojtsy41: change-server-to-website-1861930-41.patch queued for re-testing.
Comment #49
gábor hojtsy@tstoeckler: how do you like this code, especially the newly added function? :) It looks to me it is as good as it can get, given the functions it uses...
Comment #50
yesct commentedsince this issue was about replacing translation server with translation website, I think we have done that.
I made another issue to handle any specifics about distributions (since that was pre-existing problem anyway). #2143615: No "Drupal" in UI text -- config_translation, content_translation, locale, language, and translation server parts of installer
(Also removed the summary of the summary... didn't know issues could even have that. seemed to be the same as the beginning of the issue summary anyway. And dont want people updating the summary of the summary instead of the issue summary itself... like I almost just did. See #2142525: Disable the issue summary summary)
Comment #51
yesct commentedI went to check if this still applied and read it through again.
And ran into some questions.
Why was the function added in #35?
Why not just use drupal_install_profile_distribution_name() ?
Is the @see there because drupal_get_distribution_name() gets its code from there?
Also, the summary on drupal_get_distribution_name()
says: Loads installation profile...
and it's irrelevant that the way it gets the name, or at least should not be first in the summary.
and @return needs a type. https://drupal.org/node/1354#return
And then, it doesn't set a default here... so I'm not sure why we mention the default.
-----
and nit:
line wrapping in the t() is weird. but https://drupal.org/coding-standards#linelength didn't give me a definitive answer.
====
patch coming.
Comment #52
yesct commentedfound a missing period also.
Note. Maybe we should strip out the profile stuff from this issue and do it in the other issue.
I'm not sure if anyone has actually tried this with a profile to see if it works.
Comment #53
yesct commentedthis is just the website/server change.
replacing Drupal with profile is more tricky and I suggest should be in the other issue.
also complicating that is #1386394: Add back the ability for install profiles (or at least distros) to pre-select a language or modify the language-selection screen
Comment #54
gábor hojtsy@tstoeckler asked for the profile stuff in #2/#4. The drupal_install_profile_distribution_name() code is install only (see https://api.drupal.org/api/drupal/core%21includes%21install.inc/function...), although it may work on a runtime site as well, it is placed in install.inc which is not loaded on a runtime site. So if we want to display the distro name later on, there seems to be no function for that.
Not doing the distro name change would basically ignore #2/#4. It is fine to break this up even more but its already not a big change and mostly text anyway.
I thought this would be easy to do, and its definitely not worth our time debating this and making it hard for ourselves, so removing from our sprint to keep us focused on more important things.
Comment #55
sunInstead of addressing #2143615: No "Drupal" in UI text -- config_translation, content_translation, locale, language, and translation server parts of installer later on...
Can't we simply remove the word "Drupal" from all of these instances?
That way, the completely internal detail of what exact translation server/website is used for downloading translations by an installation profile or distro is no longer an issue.
It's unnecessary repetition to begin with: You know that you are installing this product called "Drupal" or "OpenAtrium" or whatnot. It uses a translation server/website. Isn't it obvious that this (whichever) translation server/website belongs to the product?
I assume that the prefix "Drupal" only exists to clarify that the referred to translation server/website is external; i.e., it is not something that exists on your local website.
If that is the intention, then let's simply call it "external translation website"
(or "server", or "service", or perhaps ideally "repository" — not sure whether I agree that "website" is really the appropriate term)
That would be the most elegant solution IMHO.
On top, it would completely eliminate the need for distros/install profiles to replace a whole bunch of strings (+ translate into a dozen of languages), just to conditionally remove/replace the word "Drupal" — conditionally, because that depends on whether l.d.o is used or not.
Comment #56
sunAlright, attached patch implements #55. I've kept the proposed wording of "website", but consistently using "external translation website".
In addition, wording in the very first installer screen is simplified and reduced to the max:
Before:
After:
Despite the changes in this patch, the user interface is apparently still littered with the string "Drupal" all over the place. We can re-purpose #2143615: No "Drupal" in UI text -- config_translation, content_translation, locale, language, and translation server parts of installer to fix/remove all of the other instances.
Also (this has been discussed before in here), the system does not allow installation profiles to supply a custom translation website URL yet. We will need to add a new installation profile .info.yml property to support that. However, that's a separate issue.
Comment #57
gábor hojtsy@sun: Good stuff! I like the proposed changes. Shorter explanations meaning the same thing are useful in general anyway :) Recategorizing as user interface text to hopefully get review from the group shepherding that. Needs issue summary update to summarize the current proposal.
Comment #58
gábor hojtsy#2091459: Update hook_help for Locale module deals with the same Drupal translation server question, I posted there too.
Comment #59
jhodgdonComing from the issue in #58...
So, for accessibility, the link text "downloaded" is not good link text. The link needs a title describing where it's going ("downloaded", the link text, doesn't), and then you are back to the problem of what to call the server.
So the patch in #56 is not OK.
Comment #60
kristen polI just installed D8 for the first time in awhile and think that it would be better UX if the "Drupal Translation website" link used target="_blank" so that it opens in a new window. I think it is confusing to open in the same window. Can we add this to this patch or does it have to be its own issue? Thanks!
Comment #61
nonsieHere's a re-roll of #56 with #59 and #60 included. It also updates the copy for help text implemented in #2091459: Update hook_help for Locale module to reflect the change from "Drupal translation server" to "external translation website"
Comment #62
nonsieStatus change
Comment #63
kbasarab commentedOnly see one technical issue in #61.
Trailing whitespace on this line.
Comment #64
jhodgdonnonsie: Also when you make a new patch on an existing issue, please include an "interdiff" file. Thanks!
https://drupal.org/documentation/git/interdiff
Comment #78
smustgrave commentedThank you for creating this issue to improve Drupal.
We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.
Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!
Comment #79
smustgrave commentedWanted to give this one more bump before closing
Comment #80
penyaskito"Drupal translation server" is still used in HEAD: https://git.drupalcode.org/search?search=%22Drupal+translation+server%22...
Comment #82
nod_Updated search link : "Drupal translation server"
Comment #83
sivaji_ganesh_jojodae commentedWorking on this issue.
Comment #85
sivaji_ganesh_jojodae commentedA draft MR has been created for this change. Kindly review and share your feedback.
Comment #86
smustgrave commented@ sivaji_ganesh_jojodae please read the whole ticket and tags. This was tagged for a summary update which was skipped.
Also please verify pipeline is passing too before review.
And have to ask did you review the changes or take the last patch for an MR? Will help with the review process
Thanks