WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#13198 closed enhancement (fixed)

Typo in Twenty Ten

Reported by: zeo Owned by: nbachiyski
Milestone: 3.0 Priority: normal
Severity: normal Version: 3.0
Component: Themes Keywords: has-patch
Focuses: Cc:

Description

Minor typo in Twenty Ten's comment for author meta description. "description" mistakenly spelled as "decscription".

Attachments (14)

twentyten-typo-comment-description.patch (1.9 KB) - added by zeo 4 years ago.
twentyten-searchform-add-trailingslash-home_url-path.patch (747 bytes) - added by zeo 4 years ago.
Add trailing slash to home_url path in 2010's searchform.php form
twentyten-edit_comment_link-spacing-text.patch (1.3 KB) - added by zeo 4 years ago.
Twenty Ten: Removes unnecesary edit_comment_link() space,  , parameter. Pingback use string (Edit), like comment.
twentyten-nbsp-space.patch (2.8 KB) - added by zeo 4 years ago.
Twenty Ten, don't need   since it's in a single line. Normal spacing should work.
twentyten-use-get_search_query-for-search-title.patch (754 bytes) - added by zeo 4 years ago.
Twenty Ten: Use get_search_query() for search title.
twentyten-coding-standard-spacing-comment-loop.php.patch (1.8 KB) - added by zeo 4 years ago.
Coding standard? Add some space to comment in file: loop.php
twentyten-add-RSS-meta-sidebar.php.patch (879 bytes) - added by zeo 4 years ago.
Twenty Ten: Follow standard theme guideline, expose RSS. Add under sidebar Meta.
twentyten-remove-extra-spacing-single.php.patch (971 bytes) - added by zeo 4 years ago.
Twenty Ten: Remove extra spacing in single.php
twentyten-add-space-edit-link-attachment.php (733 bytes) - added by zeo 4 years ago.
Twenty Ten: Add space before edit-link span tag in attachment.php.
twentyten-white-spaces.diff (17.6 KB) - added by markmcwilliams 4 years ago.
I went on the hunt for a bit of everything, some coding standards, and some removal of white spaces at the end! I could have done a few things zeo pointed out already? :)
twentyten_printf_fixes.patch (4.2 KB) - added by dimadin 4 years ago.
Fixing some printf i18n issues
13198-attachment.php.patch (35.4 KB) - added by zeo 4 years ago.
Miscellaneous. Whitespace, coding standard?, enhancement, i18n
13198-no-need-escape-invalid-param.patch (1020 bytes) - added by zeo 4 years ago.
Don't need to escape 2 times? Invalid parameter. see: esc_html($text)
13198-wrap-arrow-with-span-meta-nav.diff (3.4 KB) - added by zeo 4 years ago.
Add some meta-nav love. Wrap the left/rightward arrow entity with span to match elsewhere.

Download all attachments as: .zip

Change History (49)

comment:1 nacin4 years ago

Hi zeo,
Since you're finding quite a bit of minor things in 2010, I'm going to leave this ticket open after commit and any additional patches can go here. Just include a comment in addition to the patch, to trigger a notification.

Thanks for your help.

zeo4 years ago

Add trailing slash to home_url path in 2010's searchform.php form

zeo4 years ago

Twenty Ten: Removes unnecesary edit_comment_link() space,  , parameter. Pingback use string (Edit), like comment.

comment:2 zeo4 years ago

For this patch: http://core.trac.wordpress.org/attachment/ticket/13198/twentyten-edit_comment_link-spacing-text.patch, is it appropriate to have same Edit text for both comment and pingback.

Also since everything is in one single line, I think the used of   is not needed. Since the default parameter for $before, $after is empty we don't to pass anything.

Feedback? Thoughts?

zeo4 years ago

Twenty Ten, don't need   since it's in a single line. Normal spacing should work.

zeo4 years ago

Twenty Ten: Use get_search_query() for search title.

zeo4 years ago

Coding standard? Add some space to comment in file: loop.php

comment:3 ryan4 years ago

(In [14389]) Typo fixes. Props zeo. see #13198

comment:4 ryan4 years ago

(In [14391]) Coding standard cleanups. Props zeo. see #13198

zeo4 years ago

Twenty Ten: Follow standard theme guideline, expose RSS. Add under sidebar Meta.

comment:5 zeo4 years ago

devs, if any of the patches above is rejected do tell.

zeo4 years ago

Twenty Ten: Remove extra spacing in single.php

comment:6 zeo4 years ago

In reply to nacin:
"Two spaces in a row don't render as two. For that, you need to use at least one non-breaking space."

So then why 2 of this is different? Both are in the same row.

<?php edit_comment_link( __( '(Edit)', 'twentyten' ),'  ','' ); ?></div>
<?php edit_comment_link ( __('edit', 'twentyten'), '&nbsp;&nbsp;', '' ); ?></p>

I think rather than using "an empty" non-breaking space or space, add some span tag with class edit-link:

<?php edit_comment_link( __( '(Edit)', 'twentyten' ), ' <span class="edit-link">', '</span>' ); ?></div>

zeo4 years ago

Twenty Ten: Add space before edit-link span tag in attachment.php.

markmcwilliams4 years ago

I went on the hunt for a bit of everything, some coding standards, and some removal of white spaces at the end! I could have done a few things zeo pointed out already? :)

comment:7 markmcwilliams4 years ago

It would appear I might have covered a couple of patches zeo has already attached, so I guess we're both thinking along the same wavelengths!

comment:8 zeo4 years ago

markmcwilliams,

yeah, but the deeper i go more issue arise especially with this "coding standard" thingy etc. There's no specific guidelines AFAIK. space, whitespace, non-breaking space, tab, newline etc.

I issue a separate ticket #13243 but got closed.

comment:9 nacin4 years ago

(In [14433]) Twenty Ten spacing and string cleanups. props zeo, markmcwilliams. see #13198. Add a twentyten_credits filter, props nathanrice, fixes #12804. Also clean up the generator/credits string.

comment:10 nacin4 years ago

  • Keywords has-patch 2nd-opinion removed
  • Resolution set to fixed
  • Status changed from new to closed

Ok.

I have gone through each individual patch and committed I think everything that I felt should be committed. Everything else I either missed, or deliberately skipped. (I skipped the RSS feed sidebar patch.)

In the future, please, as you find more things, add onto the patch. The most recent patch on the ticket ideally should have contained all previous fixes as well, at least per contributor.

Also, I removed &nbsp; from continue reading, though the idea was not to wrap whitespace there. Ideally, we should be using white-space:nowrap. However, we did not use nbsp consistently (half "Continue reading" links didn't have them), so I simply removed them.

Please re-open if you find anything else like this before 3.0 is shipped (no need to spawn new tickets). Thanks for your help.

comment:11 dimadin4 years ago

  • Cc dimadin added
  • Keywords has-patch added
  • Resolution fixed deleted
  • Status changed from closed to reopened

My patch fixes several printf i18n issues. But there are some left in loop.php:

  • "Tagged "
  • "Posted in "
  • comments_popup_link and plurals (see #13187 )

dimadin4 years ago

Fixing some printf i18n issues

comment:12 nacin4 years ago

  • Owner set to nbachiyski
  • Status changed from reopened to assigned

Yeah, we have some pretty ugly strings in Twenty Ten still.

The general guideline is manageable strings and as few HTML tags as possible. Going through the diff:

  • attachment.php change is fine. (removing space)
  • footer.php -- I don't like this. (See guidelines )
  • single.php diff 1 -- this is really ugly either way. maybe Nikolay can advise.
  • single.php diff 2 -- We may need to add context to that, such %s is defined as an author specifically.
  • single.php diff 3 -- Looks okay. Don't think we need context here.

I'm going to assign this to Nikolay.

comment:13 dimadin4 years ago

  • footer.php - well, I don't like current solution either since it's not so i18n friendly
  • single.php diff 1 - we already have that code in loop.php, I just copied it (and tested, of course)

comment:14 nacin4 years ago

single.php -- Fair enough, it looked familiar. I let Nikolay know about this ticket so he can do a review of all Twenty Ten strings.

footer.php will have two relevant strings: "Semantic Personal Publishing Platform" and "Proudly powered by %s". I don't see how that's not i18n friendly, aside from not also adding contexts of "WordPress" to both of them (which I think is unnecessary).

comment:15 zeo4 years ago

About footer.php I think it's better to have:

Proudly powered by <a href="http://wordpress.org/" title="Semantic Personal Publishing Platform" rel="generator">WordPress</a>.

zeo4 years ago

Miscellaneous. Whitespace, coding standard?, enhancement, i18n

comment:16 nbachiyski4 years ago

(In [14523]) I18n fixes in Twentyten footer. Props zeo, see #13198

comment:17 nbachiyski4 years ago

(In [14532]) Twentyten attachment.php copy and i18n fixes. Props dimadin and zeo. See #13198

  • Removed the Post a comment link, since it's not present on any other single post page
  • The text and link to the full-size image weren't clear and hard to i18n. Changed to Full size is 3872 × 2592 pixel, where the size is the link
  • Whitespace fixes

comment:18 nbachiyski4 years ago

(In [14533]) Move vcard span out of the translatable string. See #13198

comment:19 nbachiyski4 years ago

(In [14535]) I18n fixes for header.php in Twentyten. Props zeo. See #13198

comment:20 doc_nl4 years ago

More string errors in loop.php:141,144 single.php:27,29,44,47

whitespaces before " which gives problems with l18n and glotpress

comment:21 nbachiyski4 years ago

(In [14541]) Clean up loop i18n. Props zeo, see #13198

  • Take out tags from strings
  • Extract two common and complicatd strings into functions
  • Whitespace

comment:22 nbachiyski4 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [14544]) Various twentyten i18n fixes. Props zeo, dimadin. Fixes #13198

comment:23 doc_nl4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Loop.php:122 stil has 'Tagged ' which breaks i18n

comment:24 nbachiyski4 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [14552]) Do not cut the Tagged <tags> phrase in half. Fixes #13198

comment:25 zeo4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

the get_template_part() block need to be indent or tab. My previous attachment.php patch contains the fix plus few other more that u left out.

For example, see twentyten category.php vs tag.php

And also: comments.php line 19.

Sorry that my patch is combined in one file and not clear in some part, nacin asked me to ;)

comment:27 nacin4 years ago

(In [14688]) Remove unnecessary and out-of-place Twenty Ten filter. Also, tabs not spaces. see #13198.

comment:28 follow-up: nacin4 years ago

I have looked at cancel_comment_reply_link. I don't see much wrong with small there honestly, it's rather semantic for that usage. I would not want to add arguments to a core function this late in the dev cycle, either. Let's stick with what we got.

Anything else we've missed here or anything else you find, please create a new patch. It's tough to fix based on line numbers referenced in comments or what we didn't commit in a previous patch, but really really easy to commit new patches.

comment:29 in reply to: ↑ 28 zeo4 years ago

Replying to nacin:

I don't see much wrong with small there honestly, it's rather semantic for that usage.

I usually can agree with you but not on this matter. It's semantically wrong. The small element spec has changed (HTML4 vs HTML5). Wrapping it inside h3 heading is the worst. This is how the document outline looks like:

<heading>Leave a Reply Cancel reply</heading>

Like I suggest in #icantremembertheticketno moved the cancel_comment_reply_link() outside heading. Put it somewhere like next to Submit button. A good example would be like the placing of Discard button used in Google Groups.

If can't make it in 3.0, will open again for discussion in future release.

zeo4 years ago

Don't need to escape 2 times? Invalid parameter. see: esc_html($text)

comment:31 ryan4 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Resolving this ticket as fixed. Please open new tickets for additional issues.

comment:32 nacin4 years ago

(In [14791]) Better escaping in 2010 attachment.php. fixes #13198, see [14789].

zeo4 years ago

Add some meta-nav love. Wrap the left/rightward arrow entity with span to match elsewhere.

comment:33 zeo4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:34 nacin4 years ago

(In [14936]) Add textdomain. see #13198.

comment:35 nacin4 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.