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.As seen on Firefox 7:
Not sure when that started happening. Doesn't happen in 7.x.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | block-test-only.patch | 513 bytes | jthorson |
FAILED: [[SimpleTest]]: [MySQL] 33,282 pass(es), 1 fail(s), and 0 exception(es). View | |||
| #13 | block-test-with-fix.patch | 1.66 KB | jthorson |
PASSED: [[SimpleTest]]: [MySQL] 33,265 pass(es). View | |||
| #12 | block-test-only.patch | 513 bytes | webchick |
FAILED: [[SimpleTest]]: [MySQL] 33,282 pass(es), 1 fail(s), and 0 exception(es). View | |||
| #12 | block-test-with-fix.patch | 1.66 KB | webchick |
PASSED: [[SimpleTest]]: [MySQL] 33,277 pass(es). View | |||
| #11 | block.diff | 1.63 KB | webchick |
PASSED: [[SimpleTest]]: [MySQL] 33,275 pass(es). View | |||
Comments
Comment #1
webchickThanks to the magic of git bisect, I discovered that this bug is caused by #918808: Standardize block cache as a drupal_render() #cache. Cross-linking both issues.
Comment #2
reglogge CreditAttribution: reglogge commentedsub
Comment #3
moshe weitzman CreditAttribution: moshe weitzman commentedI'll take this ... IMO this is about as mild a UI bug as one could imagine, and it occurs when we are about 2 years from release.
Comment #4
moshe weitzman CreditAttribution: moshe weitzman commentedPosted in the wrong issue, copied by chx.
Comment #5
chx CreditAttribution: chx commentedComment #6
webchickw00t! That takes care of it. Thanks!
Not sure if we need tests for this or not. I spent about an hour trying to write a holistic test that spun through all of the enabled blocks, checked if they had visible content when rendered, and if not, asserting that they didn't show up on the page. After 3 attempts I gave up; our block API sucks. :\
So tentatively moving to RTBC here, but we'll see what catch says.
Comment #7
chx CreditAttribution: chx commented$this->assertNoRaw('block-system-help')
Edit: yes, that's the whole test. wrap it in a class with getinfo and a test function but that's all you need to do, check the lack of that string just after install. Alternatively you can sneak it into another test to avoid a whole Drupal install for a single assert.
Comment #8
webchickYeah, that was my first attempt, but that's not really a test. It would prevent this particular problem from happening again, but the real bug is that the logic to prevent empty blocks from showing was broken.
Comment #9
moshe weitzman CreditAttribution: moshe weitzman commentedI think the proposal is a reasonable test for 'prevent empty blocks from showing'.
Comment #10
webchickSorry, accidental cross-post there. Tagging.
Comment #11
webchickOh, really? OK then...
Comment #12
webchickHow about one where I don't take you quite so literally, and include an assertion message? :P
Two patches. One with just the test so you can see it failing, the other one with the test + patch.
Note that the patch resides in an overall test function called testBlockVisibility(), so it seems to be in an appropriate place.
Comment #13
jthorson CreditAttribution: jthorson commentedRe-testing.
Comment #14
chx CreditAttribution: chx commentedWell this means this needs a change node then because of what happened to book.
Comment #15
bfroehle CreditAttribution: bfroehle commentedMarked #1299022: Block module should not print empty elements as a duplicate.
Comment #16
catchThis looks fine. I'm a bit concerned about adding a test that will continue to pass even if we don't have a help block in core any more, but can't think of a nice generic way to test this either that wouldn't be a pain. Committed and pushed.
The API change should be documented as part of #918808: Standardize block cache as a drupal_render() #cache, so putting straight to fixed.