Closed (fixed)
Project:
Drupal core
Version:
7.x-dev
Component:
Bartik theme
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
11 Feb 2013 at 09:19 UTC
Updated:
17 Nov 2014 at 05:54 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
hass commentedComment #2
hass commentedD7 version
Comment #2.0
hass commentedUpdated issue summary.
Comment #3
hass commentedComment #3.0
hass commentedUpdated issue summary.
Comment #4
yesct commentedthis 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.
Comment #5
yesct commentedsome 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.
Comment #5.0
yesct commentedUpdated issue summary.
Comment #6
yesct commentedhaving before and after screenshot would be nice.
Comment #7
hass commentedScreenshots 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.
Comment #8
yesct commentedAh, I thought it might have wider effects.
steps to test:
screenshots below anyway.
before:
after:
---
still totally rtbc.
---
next: locate other css to lowercase changes. are there any more?
Comment #9
hass commentedNope, or at least I have not found any other.
Comment #10
webchickThat 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.
Comment #11
David_Rothstein commentedYeah, 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...
Comment #12
webchickHm. 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.
Comment #13
hass commentedWe are not yet ready in D8 with the ucfirst changes... There are still a lot to change. :-)
Comment #14
sunHm.
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.
Comment #15
hass commented@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.
Comment #16
hass commentedA better title may be - Bartik theme: Don't change capitalization of translatable strings with CSS to "fix" english capitalization
Comment #16.0
hass commentedUpdated issue summary.
Comment #16.1
yesct commentedclarified 8.x was commited and 7.x needs before and after screenshots
Comment #17
yannickooTagging
Comment #17.0
yannickooadded link to contributor task doc to allow new contirbutor to join in.
Comment #18
mgiffordIt's a simple one line CSS patch that's already been brought into D8.
Before:
After:
Comment #20
hass commentedReattach #2 patch to prevent the bot fail.
Comment #22
dcam commented20: bartik_Don_t+change+capitalization+of+translatable+strings-D7.patch queued for re-testing.
Comment #23
mgiffordComment #26
mgiffordComment #29
dcam commentedComment #32
dcam commentedComment #35
dcam commentedComment #38
dcam commentedComment #40
hass commentedWhy is the status permanently flipping?
Comment #41
mgiffordOr for that matter, why aren't these RTBC issues being brought into Core?
Comment #42
hass commentedI have no idea why it take months to commit a one line css change...
Comment #43
David_Rothstein commentedCommitted 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)...