Problem/Motivation

Our translators cannot control ucfirst (capitalization).

Background: #421118: [Meta] Standardize capitalization on actions and #1496418: Views: Don't change capitalization of translatable strings with CSS

Proposed resolution

Remove the css that changes case.

Remaining tasks

  • (done!) 8.x committed in #10
  • Add before and after pictures of 7.x in several representive pages (See contributor task doc for information on how to add screenshots of an patch: http://drupal.org/node/1859584)

User interface changes

Changing case displayed (not source of t() ).

API changes

No API changes.

Files: 
CommentFileSizeAuthor
#20 bartik_Don_t+change+capitalization+of+translatable+strings-D7.patch583 byteshass
FAILED: [[SimpleTest]]: [MySQL] 40,864 pass(es), 27 fail(s), and 26 exception(s).
[ View ]
#4 drupal-Bartik_theme_Don_t_change_capitalization_of_translatable_str-1913958-0.patch584 bytesYesCT
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-Bartik_theme_Don_t_change_capitalization_of_translatable_str-1913958-0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

hass’s picture

Issue tags:+needs backport to D7
hass’s picture

Version:8.x-dev» 7.x-dev
StatusFileSize
new583 bytes
PASSED: [[SimpleTest]]: [MySQL] 39,823 pass(es).
[ View ]

D7 version

hass’s picture

Issue summary:View changes

Updated issue summary.

hass’s picture

Title:Don't change capitalization of translatable strings with CSS» Bartik theme: Don't change capitalization of translatable strings with CSS
hass’s picture

Issue summary:View changes

Updated issue summary.

YesCT’s picture

Version:7.x-dev» 8.x-dev
StatusFileSize
new584 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-Bartik_theme_Don_t_change_capitalization_of_translatable_str-1913958-0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

this is confusing to have D7 be the most recent patch. changing back to 8.x, and re-uploading the D8 patch. the initial d7 patch might be helpful later, but lets wait on further work on D7 until after the D8 one gets committed.

this is the same patch as in the issue summary, just renamed.

YesCT’s picture

some screenshots of places this has an effect would be helpful in filing follow-up issues that might be needed to change the case actually for things, and to decide if that needs to be done at the same time as removing this css.

YesCT’s picture

Updated issue summary.

YesCT’s picture

Status:Needs review» Reviewed & tested by the community
Issue tags:+Needs screenshots

having before and after screenshot would be nice.

hass’s picture

Screenshots are not possible since Devel cannot create articles and comments. I'm also not able to create a comment manually. The save button seems to be defect.

YesCT’s picture

StatusFileSize
new216.29 KB
new303.8 KB

Ah, I thought it might have wider effects.

steps to test:

  • create an article
  • comment on the article

screenshots below anyway.

before:

before.png

after:

after.png

---
still totally rtbc.

---
next: locate other css to lowercase changes. are there any more?

hass’s picture

Nope, or at least I have not found any other.

webchick’s picture

Version:8.x-dev» 7.x-dev

That does indeed seem fairly arbitrary.

Committed and pushed to 8.x. Marking down to D7.

#2 has the D7 patch. This is a user-facing change for D7, so not sure if David will go for it, but it's pretty subtle so shouldn't offend people too badly, and I can see how this is confusing for translators.

David_Rothstein’s picture

Yeah, I think we can do that for Drupal 7. It's a minor change and seems like the only way to fix the translation bug.

However, for Drupal 8 we can break strings, so shouldn't we also change the actual string to be lowercase there? Although minor, I'm pretty sure having it be lower-case was an intentional design decision (introduced in #819214: The "#" marking comment permalinks is just weird and untranslatable according to git blame).

For example, you can see in the above screenshot that having "permalink" capitalized conflicts with having "new" lower-case...

webchick’s picture

Hm. I'd say that's maybe a problem with "new" that we should open a follow-up for. The rule is "Sentence capitalization" for all user-facing strings, afaik.

hass’s picture

We are not yet ready in D8 with the ucfirst changes... There are still a lot to change. :-)

sun’s picture

Status:Reviewed & tested by the community» Needs review

Hm.

There are two independent issues and claims here, and I agree with one, but strongly disagree with the second:

1) Built-in system + base + module default CSS MUST NOT contain text case transformations.

2) Theme CSS SHOULD NOT contain text case transformations.

The actual bug report that is being claimed here refers to 1). That makes perfect sense.

However, the second part won't fix. Themes are and always have to be in control over all presentation. That includes core themes, no exception. If a theme decides that something should be output lowercase, then that decision is based on theme design considerations.

Attempting to ban text-transform CSS properties from all Drupal themes is a battle that you will not win.

hass’s picture

@sun: that may be a misunderstanding or the title is not 100% clear e.g. If a themer decides to upercase all table title names this is ok... Nothing to worry. But if there are single words in text it's not ok and not just to make the capitalization correct for english. This is the case here. I'm not sure how I'm able to explain it in english best why it's wrong here, but the css has removed the ability to preserve a capitalization a translater must be able to set. The title may sounds too generic first... A better example are the view action buttons. They are have been forced lowercase just to make the english capitalization vorrect what was not the way how it's done elsewhere. Here it's the same... All links have no lowercase capitalization except the permlink. Lowercasing seems more probkematic to me than uppercasing. Here it's not a design thing, too. Sooo if you'd like to design a website with all words lowercase... Just do it and risk the usability issues that come together with this.

hass’s picture

A better title may be - Bartik theme: Don't change capitalization of translatable strings with CSS to "fix" english capitalization

hass’s picture

Issue summary:View changes

Updated issue summary.

YesCT’s picture

Issue summary:View changes

clarified 8.x was commited and 7.x needs before and after screenshots

yannickoo’s picture

Issue tags:-Needs screenshots

Tagging

yannickoo’s picture

Issue summary:View changes

added link to contributor task doc to allow new contirbutor to join in.

mgifford’s picture

Issue summary:View changes
Status:Needs review» Reviewed & tested by the community
StatusFileSize
new176.02 KB
new160.57 KB

It's a simple one line CSS patch that's already been brought into D8.

Before:

After:

Status:Reviewed & tested by the community» Needs work
hass’s picture

Status:Needs work» Reviewed & tested by the community
StatusFileSize
new583 bytes
FAILED: [[SimpleTest]]: [MySQL] 40,864 pass(es), 27 fail(s), and 26 exception(s).
[ View ]

Reattach #2 patch to prevent the bot fail.

Status:Reviewed & tested by the community» Needs work
dcam’s picture

Status:Needs work» Needs review
mgifford’s picture

Status:Needs review» Reviewed & tested by the community

Status:Reviewed & tested by the community» Needs work

Status:Needs work» Needs review
mgifford’s picture

Status:Needs review» Reviewed & tested by the community

Status:Reviewed & tested by the community» Needs work

Status:Needs work» Needs review
dcam’s picture

Status:Needs review» Reviewed & tested by the community

Status:Reviewed & tested by the community» Needs work

Status:Needs work» Needs review
dcam’s picture

Status:Needs review» Reviewed & tested by the community

Status:Reviewed & tested by the community» Needs work

Status:Needs work» Needs review
dcam’s picture

Status:Needs review» Reviewed & tested by the community

Status:Reviewed & tested by the community» Needs work

Status:Needs work» Needs review
dcam’s picture

Status:Needs review» Reviewed & tested by the community

Status:Reviewed & tested by the community» Needs work
hass’s picture

Status:Needs work» Reviewed & tested by the community

Why is the status permanently flipping?

mgifford’s picture

Or for that matter, why aren't these RTBC issues being brought into Core?

hass’s picture

I have no idea why it take months to commit a one line css change...

David_Rothstein’s picture

Status:Reviewed & tested by the community» Fixed
Issue tags:+7.33 release notes

Committed to 7.x - thanks!

Also FYI, see #2246867: Intermittent "test did not complete due to a fatal error" failures when testing Drupal 7 patches and https://www.drupal.org/node/1868972#comment-8748657 (although it's definitely the case that the first is causing major annoyances with the second)...

  • David_Rothstein committed 9ebc7af on 7.x
    Issue #1913958 by hass, YesCT: Fixed Bartik theme shouldn't change...

Status:Fixed» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.