Skip to content

Make com_joomlaupdate info and download URL opening a new page#25848

Merged
HLeithner merged 4 commits intojoomla:stagingfrom
zero-24:newSite
Aug 20, 2019
Merged

Make com_joomlaupdate info and download URL opening a new page#25848
HLeithner merged 4 commits intojoomla:stagingfrom
zero-24:newSite

Conversation

@zero-24
Copy link
Contributor

@zero-24 zero-24 commented Aug 15, 2019

Pull Request for Issue raised by Michael Melson on Facebook

Summary of Changes

Make sure the download URL and info URL opens as a new page.

Testing Instructions

click the update URL or the info url

Expected result

new page opens as it is a remote site

Actual result

the current page is closed and the new page is open in the same tab.

Documentation Changes Required

none

@brianteeman
Copy link
Contributor

The reason that we did not put rel="noopener noreferrer" on these links is that we are assuming that joomla.org is safe and doesn't need this security prevention.

In addition if you insist on the links opening in a new window then you must add information that the link opens in a new window.

@zero-24
Copy link
Contributor Author

zero-24 commented Aug 15, 2019

The reason that we did not put rel="noopener noreferrer" on these links is that we are assuming that joomla.org is safe and doesn't need this security prevention.

In some cases we might use external sites like github too and IIRC it does not hurt right?

In addition if you insist on the links opening in a new window then you must add information that the link opens in a new window.

Any suggestion how? IIRC we have an css class for this? Or do you think we should do it different?

@brianteeman
Copy link
Contributor

No doesnt hurt

try something like < span class = sr-only > opens in a new window </ span

@zero-24
Copy link
Contributor Author

zero-24 commented Aug 15, 2019

try something like < span class = sr-only > opens in a new window </ span

Do we already have an example / string in core for this so we are reinventing the wheel here also does this need to go in staging as this is a PR against staging?

@Quy
Copy link
Contributor

Quy commented Aug 15, 2019

@zero-24
Copy link
Contributor Author

zero-24 commented Aug 15, 2019

Please check the latest commits @brianteeman @Quy

@Quy
Copy link
Contributor

Quy commented Aug 19, 2019

I have tested this item ✅ successfully on ecc9606


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

1 similar comment
@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on ecc9606


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

@ghost
Copy link

ghost commented Aug 20, 2019

Status "Ready To Commit".

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Aug 20, 2019
@HLeithner HLeithner merged commit ce3b8ea into joomla:staging Aug 20, 2019
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 20, 2019
@HLeithner
Copy link
Member

Thank you for making Joomla ux better.

@HLeithner HLeithner added this to the Joomla! 3.9.12 milestone Aug 20, 2019
@zero-24 zero-24 deleted the newSite branch August 20, 2019 14:19
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.

6 participants