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.Removed
"IP addresses listed here are blocked from your site before any modules are loaded. You may add IP addresses to the list, or delete existing entries."
- Because, the information that ip adresses are blocked before any modules are loaded, only holds value to people who understand the implications of not doing that.
- I may add and delete? Cool, but I think people will understand that.
"Enter a valid IP address."
- What else?
Changed
The field length from 64 to 32, unless the ip range of IP adresses grows extremely in the next 3 years. I don't see the need for it to be that long?
Want to change
I want to move the button beside the field, and move the list up. Not sure yet how to do this.
| Comment | File | Size | Author |
|---|---|---|---|
| #34 | issue_523286_cleanupblock.patch | 6.79 KB | AaronBauman |
| #30 | issue_523286_cleanupblock.patch | 6.12 KB | anarcat |
| #24 | banned IP message.png | 29.5 KB | joachim |
| #20 | IP-config-cleanup-20.patch | 4.75 KB | aspilicious |
| #18 | IP-config-cleanup-17.patch | 4.73 KB | aspilicious |
Comments
Comment #1
ekes CreditAttribution: ekes commentedMakes sense to me. What about a title on the list of blocked IP addresses: 'Already blocked addresses', or is that obvious enough? Also is 'remove' better than 'delete' for the option against the blocked IP addresses?
Comment #2
ekes CreditAttribution: ekes commentedoh IPv6 will need the extra characters :)
Comment #3
Bojhan CreditAttribution: Bojhan commentedOke, I'll change the length (to 42) - but as far as I understand, is it unable to handle IPv6 atm. And try to incorporate the title of the list. Not sure what the standard is on remove vs delete.
Comment #4
Bojhan CreditAttribution: Bojhan commentedThere we go
Comment #6
Bojhan CreditAttribution: Bojhan commentedfixing test
Comment #7
stBorchertthis one needs review, too
Comment #8
Dries CreditAttribution: Dries commented- Looks good to me. Should 'Save' become 'Add'? If feels like 'Add' provides a little bit more guidance.
- It might still be useful to describe why you want to block an IP address and what it means.
Comment #9
Anonymous (not verified) CreditAttribution: Anonymous commentedi'm subscribing for the problem of form layout styles... there are other forms in drupal admin where there's only one form element and a submit button, but 'container-inline' isn't a good option because of what it does to labels and descriptions.
Comment #10
Bojhan CreditAttribution: Bojhan commented@Dries - yup - changing to add. I am not sure if we want to describe why you want to block an IP address. Because you can't describe the why, without explaining the what of IP addresses - which will become pretty technical.
I am still not able to do the form repositioning
Comment #12
markus_petrux CreditAttribution: markus_petrux commentedIn response from first post comment "The field length from 64 to 32, unless the ip range of IP adresses grows extremely in the next 3 years. I don't see the need for it to be that long?"
It seems it will happen. Please, see references here:
- #475140: Make sure Drupal provides support for IPv6
Comment #13
aspilicious CreditAttribution: aspilicious commentedLet me try helping bohjan.
@ #12
IpV6 consist of 128 bits, not 128 digits....
My Ipv6 for example looks like this: 2001:0:d5c7:a2d6:cf9:39e:ab3d:a90b
Shorten the field length isn't a problem.
I changed the maxlength to 40.
I also can change it to 48 or something else greater then 35.
I also changed Save to Add...
Let's give it a try...
Comment #15
aspilicious CreditAttribution: aspilicious commentedStupid tests... XD
Comment #16
aspilicious CreditAttribution: aspilicious commentedstart bot
Comment #18
aspilicious CreditAttribution: aspilicious commentedanother try...
Comment #20
aspilicious CreditAttribution: aspilicious commentedMaybe this works (stupid me)
Comment #21
aspilicious CreditAttribution: aspilicious commentedstart bot
Comment #22
joachim CreditAttribution: joachim commentedI don't see the point of "before any modules are loaded". It's just confusing.
"You may add IP addresses to the list, or delete existing entries" is just stating the obvious.
On the other hand, we could explain just what 'blocking' means -- what would a blocked IP address see? a 404? WSOD? I have no idea.
Comment #23
Bojhan CreditAttribution: Bojhan commentedAcces denied, I'd think. I'd be fine with just explaining that, do you have a sentence perhaps?
Comment #24
joachim CreditAttribution: joachim commentedHere's what a banned IP sees.
(Interesting to note that a banned IP still sees the site name and page title though... is that a bug?) (oh wait I think that was my browser caching!)
So how about:
'IP addresses listed here are blocked from accessing your site, and instead see a brief message that they are banned.'
While we're at it, I'm wondering if we should:
- change 'Save' to 'block'
- change the the 'delete' op to 'unblock'
Don't want to be deleting teh internets! ;)
Comment #25
Bojhan CreditAttribution: Bojhan commentedWe should probally keep, it to "Save" - we dont say "Create Article" either.
We can probally remove the "and instead see a brief message that they are banned." part too - not sure what that adds? Since I am left wondering what kind of brief message then? The rest seems good. If you can file a patch for that, lets put this to RTBC.
Comment #26
joachim CreditAttribution: joachim commented> We can probally remove the "and instead see a brief message that they are banned." part too - not sure what that adds?
It gives some kind of an indication of what 'banning' entails.
Comment #27
Bojhan CreditAttribution: Bojhan commentedI dont think that is necessary, it reveals no useful information other then "some message".
Comment #28
anarcat CreditAttribution: anarcat commentedtagging with "ipv6", as this touches it a bit.
for the record, 40 is a proper length limit for an IP.
Comment #29
anarcat CreditAttribution: anarcat commentednote that I submitted a patch to fix the database end of the IPv6 support thing. Maybe we want to isolate the ipv6 changes there. http://drupal.org/node/475140#comment-2554696
Comment #30
anarcat CreditAttribution: anarcat commentedI ported the patch to head (it didn't apply anymore) and fixed the message a bit. It now says:
I hope that is sufficient for you guys, no need to go bikeshedding around this message for nothing. ;) We can always change it later too.
Comment #31
joachim CreditAttribution: joachim commented+1 for the message text.
Comment #32
marcingy CreditAttribution: marcingy commented#30: issue_523286_cleanupblock.patch queued for re-testing.
Comment #34
AaronBaumanRerolled #30 @ HEAD.
Most recent failures seem to be unrelated to this issue,
but most recent patch has one hunk that failed to apply.
Comment #35
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. This doesn't complete all of Bojhan's suggestions so I'm setting this back to code needs work for follow-up patches.
Comment #36
Dries CreditAttribution: Dries commentedComment #37
Bojhan CreditAttribution: Bojhan commented