Hi,
You can just edit the tests to reflect this :) https://github.com/joomla/joomla-cms/blob/staging/tests/unit/suites/libraries/cms/html/JHtmlTest.php#L1388 They are found in this file. You can see what is passed in and what you expect to come out. If you can edit them to reflect your changes. If you're unsure let me know and I'll submit a PR to your branch to update them so they pass when I get home this evening

Brilliant! Thanks for that, I will have a go!

No worries :) Thanks for the submitting the code!!

Okay, I have added pull-requests for 7 related test files, cf. #3226 #3227 #3228 #3229 #3230

I did my very best but travis seems to be failing all my test pull-requests now!
I've got time to help out - but I still may need some assistance to get this all to work, I'm still new to this setup, as you know!

Not at all - i remember when I was new to this 18 months ago :)

Sorry it's my fault I should have been clearer what you need to do is make your pull requests to this branch. So you go to your fork of Joomla. Then you use the drop down to move to branch patch-1 then edit the test files there so all your changes are in the same place.

The reason the new PR's are failing is because this fix in this PR hasn't been applied to that branch if that makes sense? If it doesn't shout and I'll try and explain better :)

George, you are being incredibly helpful. Do (I think!) all the commits are in the same branch 'patch-1'. Should I open a new pull request with the changes, or ...?

No your completely fine in this PR. Any changes you make to the branch are automatically added to the PR :)

Okay great :D Yes, I just saw that I had missed a couple of tests.

One last test to fix :P It would appear you've decided to tweak the most well tested function in Joomla lol. Never seen this many tests that have needed changes :)

Hah. I have to say that many of the tests appear to be redundant (from where I am seeing them).. but let's not worry about rewriting the tooltip test strategy right now :D

Generally our test strategy is at best dodgy. But yeah this isn't probably the time to worry about it yet :)

okay, so i went through that file a bit more carefully.. another 1, then another 7 tests for the tooltip, all of which needed escaping..

Nice job! Don't you just love unit tests though :D

Hah! yes. I do really - but it's easy to feel aggravated by them! So once Travis gives this a clean bill of health, does it go through to some editorial board for review? Is that you?

The workflow is that once Travis is fine, testers are needed to test the patch and make sure it doesn't break anything. Everyone can test things. After testing, the results (good or bad) should be added to the tracker item.
After at least two successful tests are logged, someone from JBS will set the tracker to "Ready to Commit" and then a maintainer will have a final look over it and merge it.

Thomas, thanks!

It looks like there are some code style issues with { }:
should be for example:
else
{
$result = $title;
}
-------not
else {
$result = $title;
}

hi - ok, sorry - used to a different set of house-rules. I can make the changes as required tomorrow. I will google for, and then review, the code style documentation for Joomla!.

For your convenience: http://joomla.github.io/coding-standards/

The code style should now be good. I noticed a common pattern elsewhere in the code, which was not to include a newline when the comment was following a brace. This seems to ignore the style ruling:
"Always have a single blank line before a comment or block of comments." So I corrected that pattern. If I am in error (or if the style guide is in error) I apologise now.

Yes, codestyle is an ongoing process. We require it only for new contributions. That is only the lines you touch need to follow codestyle, you don't necessary need to fix the other ones.

If you want help improving our code, there is a codestyle initiative which is explained here: https://groups.google.com/d/msg/joomla-dev-cms/3nosYaSl3Tk/5KMXFEoMFjgJ

This seems to ignore the style ruling:
"Always have a single blank line before a comment or block of comments." So I corrected that pattern.

I would have to check what phpcs is enforcing here, but I think if the comment is after an curly brace ({ or }) we don't need a new line before it.

Thanks for that. Well, there seemed to be a good mix of 'include a newline after a brace' and not so, so I just normalised it. In future I will modify my own codestyle only, and submit separate PR for code style amends, in accordance with the initiative. Should I now revert the changes I made in this instance?

Should I now revert the changes I made in this instance?

If you can, revert it and check with phpcs and the Joomla codestyle rules. They would be here: https://github.com/joomla/coding-standards.
If it shows as error, you are welcome to leave it changed. If it passes the checks, you can revert it in the PR as well if you want.

The file now works against phpcs standard=Joomla.

However, I notice that there appears to be a conflict between the written styleguide and the Joomla standard in phpcs - namely, that the styleguide says "Always have a single blank line before a comment or block of comments." which is in direct conflict when the comment at the beginning of a control structure. phpcs errors with: "Blank line found at start of control structure" in this case.

So it seems to me that the styleguide should say "Always have a single blank line before a comment or block of comments unless the comment (or block) is at the beginning of a code structure".

I have added a PR 65 (!) to the coding-standards project. cf. joomla/coding-standards#65

Yeah I agree the written code style should be updated :)

joomla/coding-standards#66 how does that look?

I have noticed that there are a few places in Joomla code where unescaped elements are being used in attributes directly in the code, such as /components/com_users/views/profile/tmpl/edit.php
(line 84).

This breaks xml (and html) rules. Should I add these to this patch, or should I leave this patch be now?

I see that the Zend IDE is also picking this up as being broken (but it was for a different reason).

Instructions for testing
This test will highlight a single instance of the issue that the pull request addresses.

On a fresh install of Joomla 3.2.3, using 'Default English' install.

Go to home page of website (Getting Started, with blue flower).
Login using the login form

View source of the home page.
Line 66, attribute data-jmodtip is

"<strong>Edit module</strong><br />Main Menu<br />Position: position-1"

It should be

"<strong>Edit module</strong><br />Main Menu<br />Position: position-1"

cf. http://www.w3.org/TR/html-markup/syntax.html

@test @MrBenGriffin Works as described.

(But I would not use return more than once in the function. Create a return variable und return it in the end of the function. And also use elseif for better performance.)

Can be moved to RTC.