? Success

User tests: Successful: Unsuccessful:

avatar 810
810
15 Nov 2016

On the template source you will see a tab on the meta.

Before:
before
After:
after

avatar 810 810 - open - 15 Nov 2016
avatar 810 810 - change - 15 Nov 2016
Status New Pending
avatar joomla-cms-bot
joomla-cms-bot - comment - 15 Nov 2016

Please add more information to your issue. Without test instructions and/or any description we will close this issue within 4 weeks. Thanks.
This is an automated message from the J!Tracker Application.

avatar joomla-cms-bot joomla-cms-bot - change - 15 Nov 2016
Category Libraries
avatar 810 810 - change - 15 Nov 2016
The description was changed
Labels Added: ? ?
avatar 810 810 - edited - 15 Nov 2016
avatar 810 810 - change - 15 Nov 2016
The description was changed
avatar 810 810 - edited - 15 Nov 2016
avatar infograf768
infograf768 - comment - 15 Nov 2016

I have tested this item successfully on ad2fb05

Simple cosmetic change to align the <meta charset="utf-8" /> with other meta in source


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12895.

avatar infograf768 infograf768 - test_item - 15 Nov 2016 - Tested successfully
avatar laoneo laoneo - test_item - 15 Nov 2016 - Tested successfully
avatar laoneo
laoneo - comment - 15 Nov 2016

I have tested this item successfully on ad2fb05

It assumes that is always correct intended.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12895.

avatar brianteeman brianteeman - change - 15 Nov 2016
Labels Removed: ?
avatar brianteeman brianteeman - change - 15 Nov 2016
Status Pending Ready to Commit
avatar brianteeman
brianteeman - comment - 15 Nov 2016

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12895.

avatar zero-24 zero-24 - change - 15 Nov 2016
Milestone Added:
avatar Bakual
Bakual - comment - 15 Nov 2016

That is actually the wrong place as it only works for HTML5 templates. If isHtml5 is false, the double tab will still appear.
The real question is where the double tab is being generated, and the answer is simple: In the template is an initial tab:
https://github.com/joomla/joomla-cms/blob/staging/templates/protostar/index.php#L130

So you either trim the tab from the buffer before returning/echoing it or you remove the tab in the template.

avatar Bakual Bakual - change - 15 Nov 2016
Status Ready to Commit Pending
Labels
avatar joomla-cms-bot joomla-cms-bot - change - 15 Nov 2016
Labels
avatar joomla-cms-bot joomla-cms-bot - change - 15 Nov 2016
Milestone Removed:
avatar brianteeman
brianteeman - comment - 15 Nov 2016

But the code says this is only for html5


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/12895.

avatar 810
810 - comment - 15 Nov 2016

if you remove the tab on the template itself, you have the same issue, but then on template:
dd

avatar mbabker
mbabker - comment - 15 Nov 2016

You'd have to ltrim() the tab off when returning the contents. If you're not in HTML5 mode, then whatever your first tag is will still have a tab, and we really don't need to litter the method with a $isFirst variable to decide whether to include the tab on stuff.

avatar andrepereiradasilva
andrepereiradasilva - comment - 15 Nov 2016

if you are going to that, please do it for all templates, including installation

avatar mbabker
mbabker - comment - 15 Nov 2016

If you return ltrim($buffer, $tab); at the end of JDocumentRendererHtmlHead::fetchHead() that does fix it for all templates, instead of this specific fix which only works correctly if the template's in HTML5 mode.

avatar 810
810 - comment - 15 Nov 2016

Ok, updated the code.

Works fine now.
Next ocd todo: order by type

avatar jeckodevelopment
jeckodevelopment - comment - 16 Nov 2016

Can we go back to RTC?

avatar Bakual
Bakual - comment - 16 Nov 2016

Can we go back to RTC?

Without any tests?

avatar jeckodevelopment
jeckodevelopment - comment - 16 Nov 2016

I saw your "Approval". Then let's go with tests

avatar Bakual
Bakual - comment - 16 Nov 2016

Ah, that's just the new "Review" feature from GitHub ?

avatar laoneo
laoneo - comment - 17 Nov 2016

I would just do trim($buffer); then all the blancks and new lines are gone, or is this too radical?

avatar mbabker
mbabker - comment - 17 Nov 2016

We're spitting back a "formatted" HTML string, so doing that would break all of that formatting. Great if you're trying to compress the size of your HTML response but IMO not appropriate as a default behavior.

avatar Bakual
Bakual - comment - 17 Nov 2016

trim would only remove whitespace from the beginning and end of the string. But since we know for sure there is only the tab in front of it, there is no reason to let trim look for other cases as well. It's just added work for trim without added value ?

avatar dgt41
dgt41 - comment - 17 Nov 2016

@laoneo I've tried something like what you're proposing couple years ago: #3413

avatar laoneo
laoneo - comment - 18 Nov 2016

@Bakual trim does also remove tabs and new lines without a second argument http://php.net/manual/en/function.trim.php

avatar laoneo
laoneo - comment - 18 Nov 2016

@dgt41 it is just about removing the beginning and ending characters, not the whole output

avatar mbabker
mbabker - comment - 18 Nov 2016

We don't want to be trimming the ending line break from the end of the processed head, so what this patch is doing now is just fine.

avatar Bakual
Bakual - comment - 18 Nov 2016

@laoneo Exactly. tabs and newlines count as whitespace as well. ?

avatar 810
810 - comment - 18 Nov 2016

ok,then we can merge this one

avatar rdeutz rdeutz - change - 18 Nov 2016
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2016-11-18 20:38:20
Closed_By rdeutz
avatar rdeutz rdeutz - close - 18 Nov 2016
avatar rdeutz rdeutz - merge - 18 Nov 2016

Add a Comment

Login with GitHub to post a comment