Make WordPress Core

Opened 17 years ago

Closed 17 years ago

Last modified 17 years ago

#4784 closed defect (bug) (invalid)

robot meta tag needs beautification

Reported by: hakre's profile hakre Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.2.2
Component: Template Keywords: has-patch needs-testing
Focuses: Cc:

Description

Fixed a wrong indent and made this look like real xhtml by applying double-quotification.

Diff against current CSV added, tested in my own live version and looks better then before.

Attachments (1)

4784.diff (2.4 KB) - added by hakre 17 years ago.
Some functions used single quotes for markup output. Now they use double quotes as mainly the rest of all wordpress code. A tab was missing as well, same here, tabs are use within all the code for markup elements in the head-section [TYPO/SYNTAX fixed]

Download all attachments as: .zip

Change History (27)

#1 follow-up: @foolswisdom
17 years ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from 2.2.3 to 2.3 (trunk)

hakre, looks ok, but what is with the space ("tab"?) at the front? I actually find it distracting b/c it is "non-standard".

Also, the WP convention seems to mostly be to use string.string for concatenation, ie no spaces -- makes it just that much easier to quickly see it isn't a sentence.

#2 follow-up: @DD32
17 years ago

but what is with the space ("tab"?)

To indent the <meta> tag in the <head> to the same level as the rest of the Meta tags. (To "beutify" it)

#3 @Stevie212
17 years ago

Well many generated HTML tags in WP aren't "beautiful" as in they don't use quote marks, instead they use another method, however I have noticed that if I modify the meta tag to use the quote marks, it throws an error some unknown reason, so I presume Matt uses an alternative method to prevent errors.

<meta name='robots' content='noindex,nofollow' />

Should be

<meta name="robots" content="noindex,nofollow" />

However I'm not sure theres a solution to this.

#4 @Stevie212
17 years ago

Also the <link rel='archives'> does this as well, this should be changed also.

<link rel='archives' title='August 2007' href='http://localhost/wp' />

Should be:

<link rel="archives" title="August 2007" href="http://localhost/wp" />

Although I find this an irrelevant issue, I'd assume it has benefits?

#5 @JeremyVisser
17 years ago

Stevie, single quotes in attributes are perfectly legal markup. Run it through a validator and you'll see.

The reason single quotes are used is so that there aren't tons of backslashes in the PHP code. It makes it look prettier.

(Code is poetry.)

#6 follow-up: @Otto42
17 years ago

Double quotes are required in XHTML Strict, while Transitional allows either single or double quotes.

So I support changing single quote output to double quotes anywhere in the core code, since some themes may try to implement Strict.

The indentation is unnecessary though, and should be removed. Extra spaces that do nothing should not be there.

#7 in reply to: ↑ 2 @foolswisdom
17 years ago

Replying to DD32:

but what is with the space ("tab"?)

To indent the <meta> tag in the <head> to the same level as the rest of the Meta tags. (To "beutify" it)

Well then is a bit tricky b/c that is theme to theme whether they are indented. The default is not, but many are.

Aside, a tab in that context you would want to put \t .

#8 in reply to: ↑ 6 ; follow-ups: @Nazgul
17 years ago

Replying to Otto42:

Double quotes are required in XHTML Strict, while Transitional allows either single or double quotes.

Could you point me to a place where that's specified?

I've searched the specs, but as far as I can tell XML (and XHTML) allow both ' and " to encapsulate attribute values. Also, my personal blog is XHTML 1.0 Strict according to the W3 validator and it uses ' for some attribute encapsulation.

Personally I favor readable PHP sourcecode over consistent HTML. Both are machine-readable formats which aren't meant to really be read by humans anyway, but it's the PHP code I have to read and edit as a developer.

I'd opt for a wontfix.

#9 follow-up: @JeremyVisser
17 years ago

Agree with wontfix'ing. I ran my single-quoted strict markup through the validator, and it came through okay.

I can't find a reliable citation for single quotes being invalid XML markup, especially being different depending on whether it's transitional or strict.

#10 in reply to: ↑ 9 @hakre
17 years ago

Replying to JeremyVisser:
Please see XML, there is no such thing like "transitional" or "strict" with XML this would apply to XHTML only. Both XHTML 1 transitional and XHTML 1 strict are based on XML. XHTML 1 is not HTML 4.

#11 @hakre
17 years ago

Replying to Nazgul:

Replying to Otto42:

Double quotes are required in XHTML Strict, while Transitional allows either single or double quotes.

Could you point me to a place where that's specified?

I've searched the specs, but as far as I can tell XML (and XHTML) allow both ' and " to encapsulate attribute values. Also, my personal blog is XHTML 1.0 Strict according to the W3 validator and it uses ' for some attribute encapsulation.

It's not that hard to find out that Double Quotes are needed by specs:

[10] AttValue ::= '"' ([^<&"] | Reference)* '"'

http://www.w3.org/TR/2000/REC-xml-20001006#sec-well-formed

XHTML family document types are XML based, and ultimately are designed to work in conjunction with XML-based user agents.

http://www.w3.org/TR/xhtml1/#xhtml

As XML is the basis of all XHTML double quotes should be considered. Just think of you have a useragent that is XML- and not SGML-based. Since this behaviour is compatible down to SGML (which is the Basis of HTML and does support both single and double quotes), there are no backward compability Issues in using double quotes.

If someone argues that the W3C-Validator is telling something else (I read that above) should read the docs of the validator that is clearly speaking about it's limitations. That is one if it. The old Validator page made this more clear. Additionally this is plain "best practice" in (X)HTML writing.

Anyways, this is not only about the specs only. The really most biggest part of the WP-Core-Code simply uses double-quotes for the xhtml-output so I strongly argue to do it here as well.

For the Tab (\t) it was the same kind of argumentation for me: since all the other stuff in head and the default theme have a tab in front I wanted to see this inside here as well.

In general i think it is most important to have the same form if quoting usage everywhere to create a nice output and sothat devs show they care about the output. No nitty slitty witty "but oh-oh-oh it validates" - discussions anylonger please.

Replying to Nazgul:

Personally I favor readable PHP sourcecode over consistent HTML. Both are machine-readable formats which aren't meant to really be read by humans anyway, but it's the PHP code I have to read and edit as a developer.

So you mean the code in the diff is not as readable as it should? Could you please reference the WP-Coding guidelines on this one? Since the code is edited by many people a single personal opinion shouldn't make such a bold statement herein. And I would doubt that: echo " <meta name=\"robots" content=\"noindex,nofollow\" />\n"; is more readable. But I have no problem to adopt the guidelines here in my diffs.

For me this is connected to the questions wether to use single quoted strings in favor of double quoted one in php. My practise (as of many others) is to use mainly single quoted strings otherwise sometimes being lazy, and right now I do not see why to change this with my wp-diffs. As said, point me to the guilines if there are any and I take care about it.

Replying to Stevie212:

Also the <link rel='archives'> does this as well, this should be changed also.

<link rel='archives' title='August 2007' href='http://localhost/wp' />

Should be:

<link rel="archives" title="August 2007" href="http://localhost/wp" />

Added this to the list and updated the patch. motto-quote: "Gosh, it *was* already tabbed!" ;) This time I even took the oportunity to beautify the sourcecode as well.

@hakre
17 years ago

Some functions used single quotes for markup output. Now they use double quotes as mainly the rest of all wordpress code. A tab was missing as well, same here, tabs are use within all the code for markup elements in the head-section [TYPO/SYNTAX fixed]

#12 @hakre
17 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

#13 in reply to: ↑ 1 @hakre
17 years ago

Replying to foolswisdom:

Also, the WP convention seems to mostly be to use string.string for concatenation, ie no spaces -- makes it just that much easier to quickly see it isn't a sentence.

Not within that file ;). I can add a cleanup and converting them all in that file and apply it to the patch but I dunno if that's wanted. And I think that sences do not end with a space and a dot ... . <- uuups ;)

#14 in reply to: ↑ 8 @hakre
17 years ago

Replying to Nazgul:

Replying to Otto42:

Double quotes are required in XHTML Strict, while Transitional allows either single or double quotes.

Could you point me to a place where that's specified?

+1. Please add a reference to that Otto42.

#15 in reply to: ↑ 8 @Otto42
17 years ago

Replying to Nazgul:

Replying to Otto42:

Double quotes are required in XHTML Strict, while Transitional allows either single or double quotes.

Could you point me to a place where that's specified?

Well, I thought it was the case, but looking through the spec, I cannot find it.

However, as hakre pointed out above, to be well-formed XML, it needs to have double quotes. Validators do not check for well-formedness.

#16 follow-up: @Otto42
17 years ago

Additional: Nevermind, the XML spec hakre pointed to does specifically allow for single quotes around attributes. So appearantly it is okay. I thought otherwise.

hakre, that second line is important:

[10] AttValue ::= '"' ([<&"] | Reference)* '"'

| "'" ([<&'] | Reference)* "'"

#17 in reply to: ↑ 16 @hakre
17 years ago

Replying to Otto42:

My fault, my appologies for all the misleading on that Issue. I was so sure that double quotes are needed so I ony searched the specs for the matching point... - uh-oh. I'm so glad the specs are only one part of the discussion... ;) I would say then that there isn't a difference between strict and transitional in the quoting part, right?

#18 @JeremyVisser
17 years ago

Put your code in code blocks:

(like this)

to avoid getting it munged by Trac.

#19 @foolswisdom
17 years ago

  • Milestone changed from 2.3 (trunk) to 2.4 (future)

#20 @mdawaffe
17 years ago

  • Resolution set to invalid
  • Status changed from new to closed

There is no difference between Strict and Transitional in terms of quoting.

Both double and single quotes are allowed.

http://www.w3.org/TR/2006/REC-xml-20060816/#NT-AttValue
http://www.w3.org/TR/2006/REC-xml11-20060816/#NT-AttValue

Good discussion :)

#21 @Nazgul
17 years ago

  • Milestone 2.4 (next) deleted

#22 @mdawaffe
17 years ago

Oops - feel free to reopen if there's more to discuss.

#23 @hakre
17 years ago

  • Milestone set to 2.2.3
  • Resolution invalid deleted
  • Status changed from closed to reopened

Ya, so what. Did it now apply or not? I mean this could be fixed anyway because of wordpress practise not the W3Cs one. I just fixed it because wordpress normally uses the tab and double quotes. I accidently made the false argument that W3C does as well ;) So now everynody nows that W3C does not, the Issue is still open: Fix it please!

#24 follow-up: @foolswisdom
17 years ago

  • Milestone 2.2.3 deleted
  • Resolution set to invalid
  • Status changed from reopened to closed

Reclosing as invalid for now.

hakre, you say so what? But if it was a W3C issue, it would have been a very serious issue and one that we needed to share the education about.

I feel like this is a like the color of the paint shed, and normally would approve of people just getting the painting done, but it feels a little like you are trying to paint a few paint sheds all at the same, and your skills could be applied to clear bugs.

  1. Milestone should not be for a maintenance release. The first gate for changes on a maint. release is severity of the problem.
  2. The switching b/w quotes and using concat, is probably a false optimization and harder to read -- not part of the coding style guidelines anyway http://wordpress.org/docs/developer/coding-style/ .

*. Having replaced the previously attachment is a bit awkward.

My personal bias also are:

  1. Lining up assignments is definitely harder to read
  2. A tab before the robot line is theme to theme whether they are indented. The default is not, but many are. Aside, a tab in that context you would want to put \t .

I think if you want to paint these bike sheds, you should put them in separate tickets referencing this one. The phpdocs seems part like an easy sell.

#25 in reply to: ↑ 24 @JeremyVisser
17 years ago

Replying to foolswisdom:

I feel like this is a like the color of the paint shed, and normally would approve of people just getting the painting done, but it feels a little like you are trying to paint a few paint sheds all at the same, and your skills could be applied to clear bugs.

They're actually bike sheds. ;)

#26 @hakre
17 years ago

Thanks fot the Feedback, I think about putting this into places where it belongs.

Note: See TracTickets for help on using tickets.