Skip to content

Conversation

@Croydon
Copy link
Contributor

@Croydon Croydon commented Jun 4, 2014

Replace XHTML in favor of HTML5.

No guarantee that I have found and replaced all XHTML-Tags out there in the wild, but this should be (almost) complete.

Please report any bugs which may be caused by this change, especially have a closer look on the javascript functions which manipulating the source.

Thanks =)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look correct

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. I will give it a closer look when I have more time this evening or tomorrow.
On the fly it looks like the second parameter of nl2br would be the solution, we can just set is_xhtml = false and it will output html.
The question is need we support for < 5.3.0?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, all mirrors run 5.3+

Croydon added 4 commits June 5, 2014 19:57
The <tt>-tag doesn't exists anymore in HTML5
Replace <big> with <span style="font-size:1.2em"> since <big> is obsolet
in HTML5
Replace <small> with <span style="font-size:0.8em"> because the
definition of <small> is a complete other with HTML5
@Croydon
Copy link
Contributor Author

Croydon commented Jun 5, 2014

More feedback, deprecated/obsolet/redefined tags, problems?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

problems with indentation ;)

@Sobak
Copy link
Contributor

Sobak commented Jun 6, 2014

Exactly as Hannes said - inline CSS is not better than small. It is perfectly valid HTML5 tag! I also think that most of contexts where it was used are correct semantically.

@Croydon
Copy link
Contributor Author

Croydon commented Jun 7, 2014

Well it's nice to see that I can help :)
When nobody else have found something broken I would keep it that way for now.

By the way since I don't know where else I can report bugs for the website...
When you go to http://php.net it's using http://www.php.net/cached.php?* for the css files.
And because of the same origin rule it can't load the fonts then. There is already code about that in include/prepend.inc but it seems like it wouldn't work and I don't know at the moment how to fix that.
So if somebody can give that a look... :)

@Sobak
Copy link
Contributor

Sobak commented Jun 7, 2014

It looks fine for me, now. I will, however, I'll leave to review it for @bjori as he is way more experienced dev than me. I'll also leave him decision on squashing commits.

Answering to your question: proper place to report such bugs is bugs.php.net under category called "Website problem". I don't know if using fonts directly is desired behavior, but I wouldn't be surprised.

Have a nice day.

@bjori
Copy link
Member

bjori commented Jun 8, 2014

LGTM !

@Croydon
Copy link
Contributor Author

Croydon commented Jun 20, 2014

What will happen now? :)

@php-pulls php-pulls merged commit 91dd54e into php:master Jun 20, 2014
bjori added a commit that referenced this pull request Jun 20, 2014
Kill off XHTML

* Croydon/master:
  Use release_date() also in PHP4 changelog
  Typo
  Use css class instead inline style
  Revert "Replace <small> with <span style"
  Replace <small> with <span style
  Replace <big> with <span style....
  Replace <tt> with <code>
  Drop support for < 5.3 from highlight_php()
  Finally remove XHTML from .js files
  Also remove XHTML from .inc files
  Kill off XHTML
@bjori
Copy link
Member

bjori commented Jun 20, 2014

my bad, I thought you had karma.
Merged now, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants