Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.1. Gábor showed us in his presentation that the installer has a link to the proper place where the language (.po) files can be downloaded from (to be able to install D8 in other languages than English). This link should have no "target" attribute (to be consistent with others, see below).
2. The language administration screen on http://example.com/admin/config/regional/language has a download link, but this one leads the user to a page on l.d.o where s/he can't download anything from (though this link doesn't have the "target" attribute, either). This link should lead to the same page mentioned in 1.
3. The module administration screen on http://example.com/admin/modules has also a download link which leads to the module list on d.o, and it doesn't have the "target" attribute either.
So the feature request here is to fix 1. and 2.; patch is on the way.
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | d8-1751380-10-fix.patch | 2.43 KB | Gábor Hojtsy |
| #5 | d8-1751380-5-tests.patch | 1.33 KB | Boobaa |
| #5 | d8-1751380-5-tests-and-fix.patch | 3.76 KB | Boobaa |
| #1 | d8-1751380-1-tests.patch | 1.34 KB | Boobaa |
| #1 | d8-1751380-1-tests-and-fix.patch | 3.76 KB | Boobaa |
Comments
Comment #1
BoobaaAttached is a patch containing only tests and a patch with test and fix. The first one should fail (on the admin screen text), the second one should pass. The install screen text is not tested as I don't know how could it be tested.
Comment #3
BoobaaUh-oh. Am i not allowed to create a new file in the Tests directory? I have tried both the patches locally before submitting and they did not fail to apply. Now what?
Comment #4
attiks CreditAttribution: attiks commentedYour patches are deleting files, instead of adding them. I assume you created your patch backwards
must be a + as first char
Comment #5
BoobaaLooks like I had a hangover or something.
Comment #6
attiks CreditAttribution: attiks commentedPatch is looking good, since I missed the discussion with Gabor about removing the target attribute I'll leave it to him to RTBC. I personally don't like it either, but on an installer it can be handy that the link opens in a new window so the installation isn't aborted.
Comment #7
Gábor Hojtsy1. I think its pretty excessive to have a test for this piece of link in core. I don't think we do this elsewhere either. I'd just drop the test.
2. As for the target attribute, it depends I guess, the goal was not to drive people out of the installer. We might or might not assume that much savvy from people. It is true most other links and buttons will not open in new tabs (or overlays) in Drupal, so we can remove that target probably, yeah.
Comment #8
attiks CreditAttribution: attiks commentedSince the test is here, we can leave it.
Asking for a retest, but should be RTBC
Comment #9
attiks CreditAttribution: attiks commented#5: d8-1751380-5-tests-and-fix.patch queued for re-testing.
Comment #10
Gábor HojtsyI think having a test for the test's sake is not really useful. Tests run long enough :) Here is the same patch from #5 without the tests.
Comment #11
Dries CreditAttribution: Dries commentedCommitted to 8.x. Thanks.
Comment #12
Gábor HojtsyThanks! Adding tags that should in fact have been there before.