- @anoopsingh92 opened merge request.
- 🇮🇳India anoopsingh92 Rajasthan, India
I have done @Anybody, Please review the Merge request !6
Thanks - Status changed to Needs work
over 1 year ago 9:01am 10 May 2023 - First commit to issue fork.
- Status changed to Needs review
over 1 year ago 12:30pm 15 May 2023 - Status changed to Needs work
over 1 year ago 12:38pm 15 May 2023 - 🇩🇪Germany Anybody Porta Westfalica
Thanks @Grevil, looks good! I think to be 100% safe we should have a simple test here for both values to be present, if
target="_blank"
is selected.If that goes green, RTBC from my side. Code LGTM already!
- 🇩🇪Germany Grevil
@Anybody, I agree! Furthermore, we should allow testing inside issues, as the first patch applied here failed.
- Status changed to Needs review
over 1 year ago 1:44pm 15 May 2023 - 🇩🇪Germany Anybody Porta Westfalica
Testing seems to work with patch files here manually, not with MR's. Let's try! :)
- last update
over 1 year ago 4 pass - Status changed to RTBC
over 1 year ago 2:57pm 15 May 2023 - 🇩🇪Germany Anybody Porta Westfalica
Code LGTM and tests pass, thank you @Grevil! RTBC! Let's see what @larowlan says.
- Status changed to Needs work
over 1 year ago 7:18pm 18 May 2023 - 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
This is looking good, can we add an extra test case for when target is blank, but a value of rel is already set?
Thanks!
- 🇩🇪Germany Anybody Porta Westfalica
@larowlan totally makes sense! @Grevil could you add that one please? Shouldn't be too hard anymore, I hope.
- last update
over 1 year ago 5 pass - Status changed to Needs review
over 1 year ago 8:27am 22 May 2023 - Status changed to RTBC
over 1 year ago 8:54am 22 May 2023 - 🇩🇪Germany Anybody Porta Westfalica
Nice work @Grevil, this proves that the value is "nofollow" and even with target="_blank" is not "noopener"!
The comment is also important here. Thanks! - 🇩🇪Germany Anybody Porta Westfalica
FYI: https://developer.chrome.com/docs/lighthouse/best-practices/external-anc...
As of Chromium version 88, anchors with target="_blank" automatically get noopener behavior by default. Explicit specification of rel="noopener" helps protect users of legacy browsers including Edge Legacy and Internet Explorer.
So changing this to minor. @larowlan should decide if we should do this anyway or keep it as it was before and let the browsers do their work.
Perhaps @guardiola86 could check this for the major browsers: Firefox, Safari, Edge?
- Status changed to Needs review
over 1 year ago 12:36am 22 June 2023 - 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
Yes, if we can confirm Firefox is the same, I'd prefer to close this as is and avoid the extra complexity
- Status changed to RTBC
over 1 year ago 7:19am 22 June 2023 - 🇩🇪Germany Grevil
Yes, I can confirm, that Firefox also has this feature implemented in their 79 release! See https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/79#html
Setting target="_blank" on and elements implicitly provides the same behavior as also setting rel="noopener" [...]
I guess I will set this "RTBC" to confirm that this can be closed?
- Status changed to Closed: won't fix
over 1 year ago 9:39am 22 June 2023