Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
There are some context sensitive issues where translators have no idea what it should mean.
Note to committer: Please apply patch #51.
Comments
Comment #1
hass CreditAttribution: hass commentedPatch attached
Comment #2
dawehnerI really don't get that: if they don't know how a certain link works they can search for the code and somehow are able to see what it does...
Remember: A module has string freezes since it's stable release as well, why should this be changed "just" to get better meta-translatability.
Comment #3
hass CreditAttribution: hass commentedSince we have localize.d.o and l10n_update we can change buggy strings and fix them very easy every day.
As you are also german... The original string is "Cancel link". What should this mean? A link that cancel something? Well, in this case it means the link to cancel the user account. It was translated to German as "Link zum Abbrechen"... Heck - what??? :-((( it need to mean "Link zum Löschen des Benutzeraccounts" or similar, but this is NOT what the English string says, too. Feel free to find any other better and self speaking wording. There are more of these type of strings in views we need to fix :-)
You can find this string in field selection of views if you add a new user field.
Comment #4
hass CreditAttribution: hass commentedComment #5
hass CreditAttribution: hass commentedJust for the visual review of the problem, here is a screenshot that shows the problematic strings. The string User: Link to contact page is a good string and is self speaking all others not.
Comment #6
hass CreditAttribution: hass commentedHuh, there are more strings that need to be changed.
Comment #7
hass CreditAttribution: hass commentedNew patch attached.
Comment #8
hass CreditAttribution: hass commentedMade some small changes to the wording as it may cause misunderstandings.
Comment #9
hass CreditAttribution: hass commentedBetter wording. Final now.
Comment #10
hass CreditAttribution: hass commentedD8 version of this patch.
Comment #11
hass CreditAttribution: hass commentedOne of the strings from D7 was missing in the D8 patch. Fixed version attached.
Comment #12
aspilicious CreditAttribution: aspilicious commented"Link to cancel user" isn't proper english. Add a/an/the or whatever to the sentences. Happens in multiple places.
In general I agree that some strings can be improved. I'm not sure about all the "status" changes.
Comment #13
xjmLike translators, I think I'd need to see some screenshots of these in context to understand the change.
Comment #14
hass CreditAttribution: hass commentedSee #5, please.
Comment #15
xjmThanks @hass. Could we get before-and-after screenshots of all the changed strings?
Comment #16
disasm CreditAttribution: disasm commentedattached is a re-roll for VDC. locale.views.inc and new.html do not exist, so it contains all the changes in the previous patch except those two remaining.
Comment #17
disasm CreditAttribution: disasm commentedComment #18
irunflower CreditAttribution: irunflower commentedPatch #16 applied successfully. Below is the one of the changes made. Attached are some more examples.
Before Patch:
After Patch:
*I am not sure why the after image is not showing up..
Comment #19
aspilicious CreditAttribution: aspilicious commentedOk agree this is an improvement. Good to go.
Comment #20
hass CreditAttribution: hass commentedJust for the notes. Patch in #9 is for D7 to solve the issues there, too.
Comment #21
xjmChanging this string at this point in the release cycle is likely to break more translations than it helps, so removing the backport tag. See: http://drupal.org/node/1527558
The strings are a little more verbose, but also a bit clearer. We talked a bit about ideas to redesign the handler lists a little so we don't have "Comment: foo for comment" repeated over and over and over again, but probably out of scope for this issue. I think this is probably an improvement as it is.
Comment #22
hass CreditAttribution: hass commentedHeck, how often do I need to explain this. This are critical bugs for translators like me. We are NOT *able* to translate the strings properly in D7. This are bugs and therefore requires a backport! Views has no complete translation. It's clearly wrong that this will break anything. I guess this strings are not yet translated and if they are, they are wrong (like in german) and not fixable. The patch only repairs tons of broken strings.
Comment #23
xjm@hass, http://localize.drupal.org/translate/downloads?project=views shows all the translations that have already been created. If we change these strings now, any of these existing translations of the Views UI will suddenly be broken on production sites using those translations. I checked and found several translations that include "Approve link," for example; Spanish, Swedish, Dutch, Chinese... And this doesn't even take into account translations that were not contributed on l.d.o.
I agree that it is a bug that should be fixed, but backporting it is a much less clear-cut decision to make. It has other ramifications.
Comment #24
xjmAlso, @dawehner pointed out that these are potentially not only admin-facing strings because they are used as default values for field labels.
Comment #25
hass CreditAttribution: hass commentedSee #3, please. The meaning of these ultra short strings is not unique. We have translations of these strings in German too, but very many of them are simply wrong, missleading and cannot understood. #3 is the extreme case example.
In general, views is a very large module and has a lot of strings. But with 6 years drupal translation expierence I know how bad these are and how extremly difficult to translate. Views is not ready for core if it comes to translatable strings! It's far away from core string quality. There is a lot of outstanding UI review work required to make views admin/user friendly and translatable. If the maintainer likes it or not. Translators are reading Views strings and they can often not guess what a string means. I'm not alone with this problems. This is why views is very incomplete in German and I really tried to translate all top 10 modules to be 100% complete. Strings must be long enough and need to be translatable without the context they are used in.
Strings that are broken must be fixed. String freeze or not - does not matter. We can translate them on d.o within seconds and have the benefit that these are ready then D8 comes out of the door.
Comment #26
xjmAgain, it is being fixed in D8. :) The issue is RTBC.
Comment #27
catchIsn't this one "published status or admin user"? There's no 'admin status'.
Sticky is more of a flag than a status?
While we're touching this line could we put the comments above the line per coding standards?
Comment #28
xjmWell, all three are under "publishing options," and flag might be confused with the flag module. I think status is fine so that it is parallel with published and promoted.
And, for that matter, fix the grammar:
// The item as it appears in the UI.
But actually, that doesn't make any sense to me either. The comment is superfluous and misleading; let's just delete it.
Comment #29
xjmTagging novice for the cleanup.
Comment #30
esoteric1 CreditAttribution: esoteric1 commentedcleaning it up at the moment. will post a patch.
Comment #31
esoteric1 CreditAttribution: esoteric1 commentedfirst attempt.
Comment #32
esoteric1 CreditAttribution: esoteric1 commentedComment #33
esoteric1 CreditAttribution: esoteric1 commentedThe interdiff between 31 and 16:
Comment #35
esoteric1 CreditAttribution: esoteric1 commentedThe above comment with the interdiff shouldn't have had a .patch extension. oops.
Comment #36
esoteric1 CreditAttribution: esoteric1 commentedComment #37
YesCT CreditAttribution: YesCT commentedI checked the recent work by esoteric1: looking good!
The interdiff looks correct and the new patch applies when I try it locally, and the testbot likes it too.
Comment #38
webchickAll of these changes make sense, except for this one. Didn't we just *remove* valuable context to what "type" of mail we're talking about here?
Comment #39
hass CreditAttribution: hass commentedDo we have a core rule that says it need to be written as "email" or "e-mail" or "mail". I had no idea about such a rule and just sychronized them insude of views, but I think the "e-" schould be added everywhere in core.
Comment #40
xjmAgreed with #38. Our text standards indicate that "e-mail" is the correct form.
Comment #41
YesCT CreditAttribution: YesCT commentedre-rolled as patch did not apply any more.
no conflicts, it did a 3 way merge and it looked ok to me.
then, changed the published or admin status to published status or admin user to cover the feedback from @catch in #27 and Mail to E-mail according to #39 and #40.
Comment #42
hass CreditAttribution: hass commentedThere is one help text that is written as "Email" and not "E-mail". We should change this one, too
Comment #43
YesCT CreditAttribution: YesCT commentedI think the suggestion is to change this line.
Probably also would mean removing the comment on that line.
Comment #44
YesCT CreditAttribution: YesCT commentedIf there is not already, a follow-up could be opened to clean up comment standards in core/modules/user/user.views.inc
Comment #45
hass CreditAttribution: hass commentedWe do not need a follow up. Let's make it in one patch, please.
Comment #46
YesCT CreditAttribution: YesCT commentedhass, for that one line, yes, we can use this issue.
But the whole file needs clean up, that should definitely be a separate follow-up issue. We do not want to delay getting this functionality in for you.
needs work to address #43
Comment #47
mitron CreditAttribution: mitron commentedFix for #43 plus one extra change from Email to E-mail.
Comment #48
mitron CreditAttribution: mitron commentedComment #49
YesCT CreditAttribution: YesCT commentedlooks good, if the bot says green.
Comment #50
hass CreditAttribution: hass commentedE-mail
Comment #51
mitron CreditAttribution: mitron commented@hass: Good catch.
Comment #52
dawehnerThanks for pushing this forward!
If we are here already, please let's us remove this silly and more or less useless comments everywhere.
Comment #53
YesCT CreditAttribution: YesCT commentedeverywhere, or just surrounding the lines we are changing?
I say leave that for follow-up of generally cleaning up this file *everywhere*.
@mitron I'm not sure why, but that interdiff looks a little strange. How are you generating the interdiff?
Comment #54
mitron CreditAttribution: mitron commentedThis may take a little while. I'm working on cleanup of all three files.
Comment #55
YesCT CreditAttribution: YesCT commented@mitron use #1912946: Clean up comments in comment.views.inc node.views.inc user.views.inc for that.
Comment #56
mitron CreditAttribution: mitron commentedFirst pass.
@YesCT: Was making interdiff with diff old.patch new.patch. Read Creating an interdiff after the second patch was already written so no interdiff included. Plus there are really too many changes here for an interdiff to be useful.
Edit: Please ignore this patch. It belongs in #1912946: Clean up comments in comment.views.inc node.views.inc user.views.inc
Comment #57
mitron CreditAttribution: mitron commented@YesCT: Too late. What do I do now?
Comment #58
mitron CreditAttribution: mitron commentedComment #59
dawehnerLet's ignore the patch in 56 for now?
@mitron
I hope you will be able to rerole the patch in 56 once that one is on, back to RTBC.
Comment #60
mitron CreditAttribution: mitron commentedThe patch in 56 has been reposted to Clean up comments in comment.views.inc node.views.inc user.views.inc - #4.
Comment #61
YesCT CreditAttribution: YesCT commentedSo, to help the committer that comes to look at this issue: update the issue summary to point to which patch we want them to commit (51).
actually, in addition, a little more detail using the issue summary template might be nice here anyway. (http://drupal.org/node/1155816)
thanks for posting the patch on the other issue.
yep. thats why we want it be a separate issue too. :)
Comment #62
mitron CreditAttribution: mitron commentedUpdated issue summary requesting application of patch in #51.
Can someone else add more complete information about the issue to the summary? The "action items" at the end of the comment stream were clear enough to update the patch without understanding the motivation, so I'm not sure I can do this.
Comment #63
webchickThese look like good clean-ups to me, thanks!
Committed and pushed #51 to 8.x.
Comment #64
hass CreditAttribution: hass commentedThanks for fixing these bugs in core. Now we just need to backport this bugfixes.
Comment #65
YesCT CreditAttribution: YesCT commented@hass I'm sorry to say it looks like we wont be backporting this. because it will break too many live sites.
String freeze has long been in effect for 7.x
I dont know what compromise we might be able to make.
Comment #66
hass CreditAttribution: hass commentedCritical string bugs have always been fixed and this is only contrib. Most of these strings are not translated yet or are completly wrong translated. Some of these wrong strings can cause major harm (data loss) to site owners. l10n_update will manage the update. There is no blocking reason not to fix these strings in views D7. They are also only in admin section and not in the public area.
Comment #67
dawehnerThat's technical wrong. If you have a field shown with the default label, it get's exposed to the user, so if you have a view with default labels, these kind of patches would change it.
Hopefully you don't argue against that, because then you care more about strings then users, sorry but you sometimes kind of overreact :(
Comment #68
hass CreditAttribution: hass commentedI may forgotten this small detail. However, the example is still valid for what I opened the case. This is critical as the translation of
t('Cancel link')
and others is completely wrong and cannot fixed by translators as this string requires a context that it do not have - or only have now in D8.I don't understand why we need to discuss this again and again just for a minor reason that is named string freeze. We have also fixed bad and broken core strings post final release. There is zero reason to hold such patches back. I fixed thousands of strings in contrib POST final releases because they are not translatable, have bad usability or other reasons. I do not like translate a bad string and later a good string. I fix strings first and then translate them.
Views administration for beginners (and others) is bad UX (also in English), if you like it or not - doesn't change the UX and usability issues in views. Users do not understand views administration. I provided some patches for D7 in the beginning of this case, but we should re-role the last D8 patch against D7 to get these strings ready and fixed and translated in D7 times so they are ready when D8 comes out and not to forget the usability in D7 can only *win*.
Comment #69
hass CreditAttribution: hass commentedHere is the D7 version of #51.
Comment #70
mitron CreditAttribution: mitron commentedIf all the translation of these strings are wrong, then we should be able to break string freeze. It does not matter since the translations are all wrong. However, it they are wrong in two languages out of 100, then no way should we break string freeze. How can we know?
Comment #71
hass CreditAttribution: hass commentedThese strings are not translated in 100 languages. We are talking about guessed 5-10 at very maximum because they are not translatable without the context. There is no views translation around that is complete... None! German is the most complete as I know.
Comment #72
mitron CreditAttribution: mitron commentedSo then does that not make a strong argument that we CAN apply this patch to D7?
Comment #73
YesCT CreditAttribution: YesCT commentedthanks for the d7 patch.
This is just contrib in D7. But it's views, which is in a lot of sites.
Is there any way to show data for how many translations there are for these (seems that might be posible), and how many are correct or incorrect (I have a doubt)?
Maybe we can grep and get some data.
It would be nice to get another translator or admin of a D7 site to weigh in here.
is there another issue number we can look at that was a string fix in contrib (maybe in views)?
Comment #74
hass CreditAttribution: hass commented@YesCT: Try this ~100 cases or Translatable string review from my random search.
Views has also not received a string review before a release. It's not worth do discuss this from my point of view; if this can be committed or not. It must be committed.
Comment #75
tobiasbThat are just strings, no big api-changes, so lets fix the few strings in views. ;-)
Comment #76
mitron CreditAttribution: mitron commentedIf this is committed and included in the next Views release, these strings will appear in English once the new version of views is installed on a site until
1) These new strings are translated.
2) A new translation is released.
3) The translation is downloaded and installed on the site.
A quick survey of http://localize.drupal.org/translate/language drilling into views looks like these strings are already in English for most languages because the .po files are incomplete and do not include these strings.
Comment #77
YesCT CreditAttribution: YesCT commentedxjm's last objection was in comment #23
Let's see see if those objections still exist.
It's more work, but a compromise (where no side is really happy) might be to locate which strings are not translated in translations yet, and do the patch for those.
Comment #78
David_Rothstein CreditAttribution: David_Rothstein commentedI'm not sure if this helps, but note that you don't need to change the user-visible string itself in order to provide context for translators.
For example, instead of this:
You can just do this:
And not change the string at all.
(I actually wonder if this should be reconsidered for D8 as well, since some of the new strings that were added here look unnecessarily verbose.)
In any case, the reason it might not help the current discussion is that I think it still gets treated as a string breakage; if there's no translation for
t('Edit link', array(), array('context' => 'Link to edit comment'))
it looks like the Locale module will not fall back ont('Edit link')
but rather just treat it as untranslated. But I'm mentioning it anyway, in case I'm wrong.Someday we really need to find a way to make the whole system smarter, so that backporting string changes isn't so contentious... For example, I heard a while ago that Wordpress can actually keep track of the mapping between string changes, so if t('Edit link') got changed to t('Link to edit comment') at some point and if there is no translation for 'Link to edit comment' available, it knows that it should look up the translation for 'Edit link' as a backup... or something like that. We should seriously think about doing that - anyone want to create an issue and look into it? :)
Comment #79
mitron CreditAttribution: mitron commentedI don't see how WordPress could link a prior phrase to translate with an updated phrase. The function to translate in Wordpress is __('message'). If 'message' is found, it is translated. Otherwise, 'message' is returned. If in the prior version, it was __('old') and in the new version it is __('new') there is no way for the function to connect 'new' with 'old.'
http://codex.wordpress.org/Translating_WordPress
Comment #80
hass CreditAttribution: hass commentedNote that we should not use "context" if there is any other way to solve issues. Context does not scale.
Comment #81
hass CreditAttribution: hass commentedFound one more bug, but made a new case as it's easier to commit. See #1974208: Rename "Cancel" to "Cancel account" to help translators., please.
Comment #82
dawehnerThanks for creating a new issue!
Comment #83
heddntagging
Comment #83.0
heddnTell committer which patch to apply
Comment #84
Chris Matthews CreditAttribution: Chris Matthews commentedThe 6 year old D7 patch in #69 does not apply to the latest views 7.x-3.x-dev.
Comment #85
Andrew Answer CreditAttribution: Andrew Answer as a volunteer commentedPatch rerolled.
Comment #86
hass CreditAttribution: hass commentedComment #88
DamienMcKennaCommitted. Thanks everyone.