Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.0] Add target _blank to display icon link as external #34760

Merged
merged 4 commits into from
Jul 12, 2021

Conversation

Quy
Copy link
Contributor

@Quy Quy commented Jul 11, 2021

Summary of Changes

Add icon to indicate link is external.

Testing Instructions

Go to Post Installation Messages
See links with external icon.

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-4.0-dev labels Jul 11, 2021
@Quy Quy changed the title [4.0] Add target _blank [4.0] Add target _blank to display icon link as external Jul 11, 2021
@brianteeman
Copy link
Contributor

if you're going to change it then you also need to add the noreferrer stuff

@hans2103
Copy link
Contributor

I have tested this item ✅ successfully on f0adaa8


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

1 similar comment
@sandewt
Copy link
Contributor

sandewt commented Jul 12, 2021

I have tested this item ✅ successfully on f0adaa8


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

@sandewt
Copy link
Contributor

sandewt commented Jul 12, 2021

34760

Oops, missing here: noopener noreferrer

<a href="https://en.wikipedia.org/wiki/Google_Authenticator" target="_blank">Google Authenticator</a>

@hans2103
Copy link
Contributor

34760

Oops, missing here: noopener noreferrer

<a href="https://en.wikipedia.org/wiki/Google_Authenticator" target="_blank">Google Authenticator</a>

out of scope for this PR.
Language string for this post-install message comes from plg_system_httpheaders.ini

PLG_SYSTEM_HTTPHEADERS_POSTINSTALL_INTRODUCTION_BODY="<p>Joomla! comes with a built-in set of tools that help you to handle http security headers. These headers help your browser for example to protect your website from <a href='https://en.wikipedia.org/wiki/Cross-site_scripting' target='_blank' rel='noopener noreferrer'>XSS</a> and <a href='https://en.wikipedia.org/wiki/Clickjacking' target='_blank' rel='noopener noreferrer'>Clickjacking</a> attacks.</p><p>You can find more details in the <a href='https://docs.joomla.org/Special:MyLanguage/J4.x:Http_Header_Management' target='_blank' rel='noopener noreferrer'>HTTP Header Management Tutorial in the Joomla! Documentation.</a></p>"

And already contains noopener noreferrer

@sandewt
Copy link
Contributor

sandewt commented Jul 12, 2021

out of scope for this PR.

I agree.

I see that this problem also occurs with the external links under the help function.

34760-2

@sandewt
Copy link
Contributor

sandewt commented Jul 12, 2021

And already contains noopener noreferrer

I agree, apparently something went wrong for me when I upgraded to J 4.0.0-rc3 !?

@richard67
Copy link
Member

I agree, apparently something went wrong for me when I upgraded to J 4.0.0-rc3 !?

@sandewt So is your successful test still valid or not?

@richard67
Copy link
Member

I have tested this item ✅ successfully on f0adaa8


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

@richard67
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 12, 2021
@richard67 richard67 added this to the Joomla 4.0 milestone Jul 12, 2021
@sandewt
Copy link
Contributor

sandewt commented Jul 12, 2021

@hans2103 / @richard67

It concerns the plg_twofactorauth_totp.ini and NOT the plg_system_httpheaders.ini

See: missing noopener noreferrer

PLG_TWOFACTORAUTH_TOTP_POSTINSTALL_BODY="<p>Joomla! comes with a built-in two factor authentication system. It secures your site login with a secondary secret code that's changing every 30 seconds. You can use your mobile device and the <a href=\"https://en.wikipedia.org/wiki/Google_Authenticator\" target=\"_blank\">Google Authenticator</a> app to produce that code.</p><p>By selecting the button below:</p><ul><li>Joomla! will enable the two factor authentication plugins</li><li>Two Factor Authentication is going to be available for all users.</li><li>Each user can configure Two Factor Authentication in User Details.</li><li>You can always disable Two Factor Authentication plugin, or configure it for Backend usage only.</li><li>You will be taken to your user profile page where you can find more information on two factor authentication and enable it for your user account.</li></ul>"

out of scope for this PR.

So I think, this is not out of the scope

@sandewt So is your successful test still valid or not?

NO

@richard67
Copy link
Member

See: missing noopener noreferrer

PLG_TWOFACTORAUTH_TOTP_POSTINSTALL_BODY="<p>Joomla! comes with a built-in two factor authentication system. It secures your site login with a secondary secret code that's changing every 30 seconds. You can use your mobile device and the <a href=\"https://en.wikipedia.org/wiki/Google_Authenticator\" target=\"_blank\">Google Authenticator</a> app to produce that code.</p><p>By selecting the button below:</p><ul><li>Joomla! will enable the two factor authentication plugins</li><li>Two Factor Authentication is going to be available for all users.</li><li>Each user can configure Two Factor Authentication in User Details.</li><li>You can always disable Two Factor Authentication plugin, or configure it for Backend usage only.</li><li>You will be taken to your user profile page where you can find more information on two factor authentication and enable it for your user account.</li></ul>"

out of scope for this PR.

So I think, this is not out of the scope

@sandewt So is your successful test still valid or not?

NO

@Quy Should we fix this in staging or here?

@hans2103
Copy link
Contributor

Created a PR in this PR
Because discussing about it takes more time than fixing it.
Quy#3

@sandewt
Copy link
Contributor

sandewt commented Jul 12, 2021

Because discussing about it takes more time than fixing it.

😃

@richard67
Copy link
Member

Well I had noticed that but I thought the reason why it has not been made in this PR was because it should be fixed in staging.

@brianteeman
Copy link
Contributor

With the language string freeze tomorrow it will need to happen here

Because discussing about it takes more time than fixing it. ;-)
@Quy
Copy link
Contributor Author

Quy commented Jul 12, 2021

#24337 discusses this issue globally for J4.

@HLeithner will have to decide if for J3.

@richard67
Copy link
Member

I have tested this item ✅ successfully on 96cb627


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

@richard67
Copy link
Member

@hans2103 Could you test again? Thanks in advance.

@hans2103
Copy link
Contributor

I have tested this item ✅ successfully on 96cb627


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

@richard67
Copy link
Member

2 good tests, so RTC is valid again.

@zero-24 zero-24 merged commit 9211583 into joomla:4.0-dev Jul 12, 2021
@zero-24
Copy link
Contributor

zero-24 commented Jul 12, 2021

Merging

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jul 12, 2021
@Quy Quy deleted the target-blank branch July 12, 2021 16:19
@HLeithner
Copy link
Member

#24337 discusses this issue globally for J4.

@HLeithner will have to decide if for J3.

+1

@sandewt
Copy link
Contributor

sandewt commented Jul 12, 2021

See: missing noopener noreferrer (occurs twice)

<a href=\"https://freeotp.github.io/\" target=\"_blank\">FreeOTP</a>

https://github.com/Quy/joomla-cms/blob/96cb627464c0f415b42c8fd4187451c45122062e/administrator/language/en-GB/plg_twofactorauth_totp.ini#L24

@Quy

[EDIT]

@Quy
Copy link
Contributor Author

Quy commented Jul 15, 2021

@sandewt This PR fixed only external links in the Post Installation Messages page. There are other instances like the ones you mentioned to be fixed in a separate PR to address #24337.

@sandewt
Copy link
Contributor

sandewt commented Jul 15, 2021

@Quy , thanks for your explanation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants