Review (and in most cases remove) use of target="_blank" attribute outside the installer or update.php

The patch does not apply. Even when i create the same patch by manually doing the change. I created the .diff extension file it applies. I am a newbie am i doing any error?

The patch applies cleanly, and works as announced.
However this only fixes the links to the date php settings. When looking through drupal core, there are lot's of places a link to an external site (mostly drupal.org / php.net and wikipedia) are shown. (For example status report page). Should we be able to fix those in one go?
This probably needs some grep magic to find them all. Some are to http / https sites, and contained in the a tag, others are escaped, and the link is added afterwards.

Changing the bug status to fixed, the patch works well ! Just to ask have you committed your local code after applying the patch.

@imrancluster we cannot push back to the core. It would be taken care by the core team. Actually, my answer was intended to suraj1074 sorry for the confusion :)

Before we go ahead and break most translatable strings in Drupal, has anyone done some research as to why this is a good idea?

From reading https://css-tricks.com/use-target_blank/ it comes up with clear examples on when target=_blank should be used. A blanket "internal and external" rule seems to make little sense.

Keen to hear some more justification on this. Also, if this is restricted to just where places where it makes the most sense, can this be targeted at 8.0.x ?

I updated the ckeditor changes

I think, it is too big to be done in one shot. It will be very difficult to debug why patch failed, because there are many files involved.
Let's chunk it into pieces with one issue for each module. Remaining files can be grouped into one last shot.
Would like to hear feedback on this.

Grammer correction in the title.

This patch opens the php manual in a new tab using jquery.

@imrancluster : Don't really have to worry about its behavior on different devices as its suppose to be behavior is clearly defined here.

@Neelanjana Das : We don't really need to have a JS snippet for this implementation. Please user target=_blank instead.

Cheers,
SwapS.

I just reload the latest passed patch against 8.1.x-dev.

Patch on #37 looks good.
Works as expected for Date module fixes.

Also , as suggested by @bhavikshah9 ; we should create separate issues for fixing it for other module (should have one primary ticket to track it to closure.)

Cheers,
SwapS.

This is generally discouraged - there's plenty of ways to open a link in a tab. Tagging for an issue summary update as to why we'd do this vs. letting people choose, and for usability review.

I don't think we've defined a standard for this. It is generally discouraged indeed. How many of these kinds of links are there in core?

The real solution for the specific case of setting a date format would be a better UI for it that doesn't rely on external documentation to make it work. Something more than a single text field :)

Hopefully @Bojhan will hop into this as we were just discussing this in IRC.

The problem is that core isn't consistent in how target=_blank is used. I don't think a blanket "all external links should open in new windows" is appropriate.

I would propose that we use this for all external links when a link is used on a form / wizard, or anywhere else user input may be lost when they navigate to a new page. I also think that in addition to target=_blank, we should add rel=external to them for improved accessibility for devices that grok this.

For sanity, I would be in favor of scoping the issues to links out to a particular site, but not by module. So, we rescope this issue to be all links out to php.net and then evaluate other sites (any maybe one issue to mop up the one-offs).

I will update the IS later with arguments for both sites.

#41, that is really unfeasible as the link is related to custom date formatters where you need to know the PHP syntax / placeholder codes; there are already standard formatters that are user-friendly. There are a decent number of these links in core out to external resources, but not a ton.

I'd say having to know the PHP syntax / placeholder codes to set a custom date format is the unfeasible part of this user interface :)

For sanity, I would be in favor of scoping the issues to links out to a particular site

That makes sense to me too.

So. It seems like there are two questions/issues here:

a) For Date/Time formatting, is it possible we could provide either a better UI or some documentation in the page, so that someone could do a custom date format without linking to php.net?

b) In general, we have links in our UI text that go to either drupal.org, php.net, or wikipedia.org (and maybe other places, but I think those three are the most common). We have no standard saying that they should or shouldn't automatically open in a different tab. Should we have a standard about it, and what should it be? Note that we need to consider accessibility and usability both when deciding on this standard. I do not know if target _blank is considered OK for accessibility.

Anyway... I don't know if we should be trying to resolve both of these on this one issue. But it seems like for (b) we need to decide on a standard and then apply it everywhere in Core. For (a) the issue would be just in the date module.

b.) Please do not make links that open a new browser window. That's what right click->open in new browser is for.

I kind of agree with you, but... how do you right click on a phone? I suppose it is something like long-press/choose from options. I don't do much browsing on my phone.

#45-a, that page las a lot of options. A lot. I am not sure if we could make it concise enough to be usable for when you have a need for a custom formatter, especially when there are a slew of standard ones.

#45-b, I am hesitant to make an issue about standardizing this. It will win Bikeshed-of-the-Year. But, I think it needs to be done.

I think that this particular issue should be just about the Date stuff, and we should open a separate one about _blank in general (even though the title of this one sounds generic, the discussion has mostly been about Date).

The accessibility pieces are essentially about user control. Opening up a new page to direct folks to a new page can be disorienting. It's a much better practice to teach users how to use their browser. It's really hard for some users to handle multiple screens (especially on a mobile device) even the browser has good support for them.

Some related resources:
https://css-tricks.com/use-target_blank/
https://developer.mozilla.org/en-US/docs/Web/API/Window/open#Do_not_use_...
http://alistapart.com/article/popuplinks

I wonder if adding a tooltip might be a better pattern here. Just to alert them that this is going outside of their Drupal site.

Postponing on #2702881: [policy, no patch] Formalize how external links are handled in core.

As a string change, this change would need to be targeted for the development minor, which is currently 8.3.x-dev.

Reading this again, I think in my comment in #46 that I thought this was generic and maybe didn't realize it was for one specific page. Also, since it's an external page (and we open all external pages on the Babson Portal sites in a new tab), I'm less against it now.

Furthermore, the external page is a reference/help for the opening page.

Remove tag until the

Interestingly, the original IS requested the opposite of the current scope and accessibility best practice. Updating the IS to avoid confusion.

This is at least wrong in core everywhere the string the documentation for PHP date formats appears, and in the empty text for the frontpage view. And maybe the announcements feed.

Better scope.

Thanks for updating this @xjm.

We do indeed want to remove the target="_blank" attribute where we can.

Tag cleanup. (We don't actually need a11y review right now, and I did the IS update.)

I guess whether or not the date format links should open a new window is debatable, because technically they could be on a multistep form. I would still lean toward removing them. We can probably have an accessibility maintainer give feedback on that once we've identified the full scope of what core does.

A novice could work on the next step of this. The next step on the issue is to search for all the uses of target="_blank" in core, and document a list of which category they fall under as I've outlined them in the issue summary. Pay careful attention to the instructions in the issue summary.

Fixed a minor typo in the issue summary.

Added a child issue #3541452: Installer and user interactions related target="_blank" in core for

4. For categories 1 and 2, a single child issue should be filed to ensure all the links warn the user that a new window will be opened.

Adding the findings around the below categories:

1. The installer or update.php
2. Other multistep forms or complex user interactions

Thank you for providing a start to this list of usages, @neerajsingh.

I performed Novice Triage on this issue. I am leaving the Novice tag on this issue because I did a review of core and found more instances of target="blank".

This list could be very long as a table and after I asked @xjm we came to a similar conclusion that a concise list format would be better in the issue summary so I added the Needs issue summary update tag.

So the tasks are

1. Find all the remaining uses of target="_blank" following the instructions in the Remaining tasks section
2. Update the issue summary and add this list into the Proposed resolution section using a format like
   

       <h4>Category</h4>
       <ul>
        <li>core/path/to/file:Line Number</li>
      </ul>
     

The Drupal Contribution Mentoring team is triaging issues for DrupalCon Nara 2025, and we are reserving this issue for Mentored Contribution during the event. After November 19, this issue returns to being open to all. Thanks!

We are working on it in DrupalCon Nara Contrib Day 2025....

We are working on it in DrupalCon Nara Contrib Day 2025.

As we have a lot of occurrences of the target="_blank" - it will be hard to manage in the issue text, I believe.

But I came up with an idea that we can prepare a merge request with the first commit that removes all occurrences of the target="_blank" in all files, and then, step by step, we will make a decision on what to keep and what to revert.

This way should me much more convenient because:
- We will have an actual dynamic list of changes.
- Each changed line will provide an easy way to look on the code around it to understand the context.
- We will be able to comment and discuss each case in a separate thread to make the final decision.

So, we in the Nara contribution room will prepare a merge request with this to start.

We are working on it in DrupalCon Nara Contrib Day 2025.

Prior the MR we worked together to build the consolidated list, we also review each file and assign the category to within the MR.

We are working on it in DrupalCon Nara Contrib Day 2025

I believe @skifdesu, @murz, @xxkiemi, and @awset did a great job reviewing and identifying the files that use the target="_blank" attribute.
I'm updating the issue status to Needs Review.

However, this still needs confirmation because creating an MR may not currently be part of the acceptance criteria. We should also verify whether we actually want to apply these changes before proceeding.

Thanks for identifying the category for each change! Now, to review those, it will actually be better in separate MR's per the remaining tasks in the issue summary. xjm set that up according to the Issue Scope Guidelines, which are to improve things like the quality of the review. It will also separate the category 3 changes which will require input from the accessibility team. Waiting on those discussion should not delay the other categories.

Therefor, setting to needs work for step 3, 4, and 5.