I just found this on a Drupal 6 site, but it appears to be the same in Drupal 7.
To reproduce:
Enable a block.
In block configuration, select "Only the listed pages".
Leave the textarea blank.
In the database, there will be absolutely nothing in the 'pages' column for the block on saving the form, which means that instead of not showing on any pages, the block actually shows on all of them.
This would be obvious in some cases, but in the case I just found, the block was executing a view, and would only actually end up getting displayed on a very small number of pages (for example the view default arguments might not get validated, or there might not be any content).
Now it's possible to just disable the block, but I'm sure people zero out this textarea when they just don't want something rendered for a while but want to leave the position and weight etc. there for later, and it seems like a reasonable expectation that the option would actually work like this.
Comment | File | Size | Author |
---|---|---|---|
#24 | 1078176-block-render-d6-24.patch | 433 bytes | Albert Volkman |
#22 | 937560-d6-22.patch | 1.17 KB | Albert Volkman |
#9 | 1078176-block-render.patch | 2.52 KB | franz |
#7 | 1078176-block-render.patch | 2.52 KB | franz |
#6 | 1078176-block-render.patch | 522 bytes | franz |
Comments
Comment #1
agentrickardThis condition should really be checked by block_admin_configure_validate(). That's a pretty simple patch. If path visibility is selected as BLOCK_VISIBILITY_LISTED, the option should not be empty.
Comment #2
barbi CreditAttribution: barbi commentedIf the path visibility is BLOCK_VISIBILITY_LISTED, shouldn't the users be able to leave the pages textbox empty? This is the equivalent of saying - Display in no pages as of now.
Comment #3
agentrickardMakes sense. Needs a test. Setting to needs review for testbot.
We could also do this with a query condition, which might be a little faster?
Comment #5
catchComment #6
franzThis was a catch.
When the user visibility was set to hidden and all other were set to visible, the new if-else clause would make it visible.
The old else clause remained on the user-visibility if clause. The new check should only disable the block in case it is BLOCK_VISIBILITY_LISTED with empty path, and not enable it otherwise.
Comment #7
franzFirst time writing a test for core, tell me if this is ok.
Comment #9
franzThe block had to be moved to a region before being set to hidden, otherwise assertions on moving the block will fail.
Comment #10
agentrickardNice. One more review and RTBC.
Comment #11
barbi CreditAttribution: barbi commentedComment #12
franzbumping this.
Comment #13
bassthiam CreditAttribution: bassthiam commentedNow block aren't displayed in any page when set to "Only the listed page" with blank textarea .
tested in drupal7.
Comment #14
franzGuess this is RTBC then
Comment #15
Dries CreditAttribution: Dries commentedThis use case is a bit funny, but I guess it is the proper fix. Committed to 7.x and 8.x. Thanks!
Comment #16
bassthiam CreditAttribution: bassthiam commentedGuess so
Comment #17
franzAh, as for the original issue, I guess this also applies to D6.
Comment #18
darrylmabini CreditAttribution: darrylmabini commentedComment #19
franzWhy "needs review" if there is no D6 patch?
Comment #20
greg.harveyComment #21
darrylmabini CreditAttribution: darrylmabini commentedSorry, my mistake - I accidentally clicked the re-test link.
Comment #22
Albert Volkman CreditAttribution: Albert Volkman commentedBackported to D6.
Comment #23
agentrickard@Albert Volkman
You uploaded the wrong patch.
Comment #24
Albert Volkman CreditAttribution: Albert Volkman commentedEpic fail.
Comment #25
agentrickardLooks good from here.
Comment #26
d.novikov CreditAttribution: d.novikov commentedPatch http://drupal.org/node/1078176#comment-5466136 worked for me.
Comment #27
agentrickardComment #28
Kristen PolRTBC++
I applied the patch and tested that a block does not show up if
Show on only the listed pages.
option is selected and thePages
textarea is empty.Comment #29
star-szrRemoving novice tag, nothing more to be done here other than a commit.