Closed (fixed)
Project:
Drupal core
Version:
x.y.z
Component:
block.module
Priority:
Critical
Category:
Bug report
Assigned:
Reporter:
Created:
4 Apr 2005 at 17:03 UTC
Updated:
15 Feb 2006 at 22:46 UTC
Jump to comment: Most recent file
When setting more than one block's title to '' (blank) Drupal dies, and doesn't catch the SQL query error nicely. The error is as follows:
user error: Duplicate entry '' for key 2
query: UPDATE boxes SET title = '', body = ' ...<cut>... in /var/www/html/drupal-cvs/includes/database.mysql.inc on line 66.
Using 4.6 build from 4/4/2005
The page ends with:
warning: Cannot modify header information - headers already sent by (output started at /var/www/html/drupal-cvs/includes/common.inc:384) in /var/www/html/drupal-cvs/includes/common.inc on line 192.
| Comment | File | Size | Author |
|---|---|---|---|
| #22 | block-fix.patch | 673 bytes | killes@www.drop.org |
| #21 | 4_0.6 blank title | 3.62 KB | robin monks |
| #20 | 4.6 blank title | 3.63 KB | robin monks |
| #18 | drupal_1.patch | 3.97 KB | robin monks |
| #11 | drupal_0.patch | 3.67 KB | robin monks |
Comments
Comment #1
robertdouglass commentedThere should be a check before the insert is attempted and if the block title already exists the user should be notified with form_error().
Comment #2
Poolio commentedthis certainly doesn't solve the problem, but I've worked around this issue by using html tags in the title field for blocks that I don't want to show a title.
Comment #3
robin monks commentedA small patch that checks to ensure the title does not already exit in the Blocks list. Also, if the field was left blank, in adds an HTML comment with the current date to the second to ensure that a duplicate title isn't created.
This is my first patch, so I probably did something wrong, but I'm willing to fix it too.
Robin Monks
MozNetwork.org
Comment #4
morbus iffI'm, personally, fine with how you've fixed the blank block title. You've really solved two problems in one: duplicate titles (which is the cause of this Issue), and the desire to have blank (or no) titles (which is the reason for this Issue) for the end user. With that said, I'm not a fan of your error message (but, then again, anal wording is my specialty, so don't think I'm picking on you):
I'd rename it to "That block title is already in use; please choose another."
The goals here:
Comment #5
chx commentedWhy display any message at all?
Comment #6
morbus iffDisplaying no message at all would only work if the user wasn't typing in ANY title whatsoever (since then we'd make one for them based on the date). But, if a user names a block "Users" one week, and then creates another block called "Users" three weeks later, than an whoopsidaisy error needs to be shown - we shouldn't modify the text randomly.
Comment #7
chx commentedWell, let's discuss the main point: why shoiuld be the title unique?
Comment #8
morbus iffAre you asking why Drupal enforces them being unique, or why, visually and to the end-user, they should be unique?
Comment #9
chx commentedI guess Drupal forces them 'cos for some reasons it is thought that for the users they should be unique.
This is not the case, however. Yes, it's confusing if they are on the same page. But I can think on circumstances where they would not be on the same page, so the force from Drupal seems unnecessary.
Comment #10
robertdouglass commentedagree with chx on the unique point. Like the functionality of adding html comments and timestamp (why timestamp? why not a block id?) if left blank if they must be uniqe. Would be in favor of making the visible title completely optional. Administrators have description to keep them separate on the admin screen.
Comment #11
robin monks commentedThis new patch no longer requires a unique value for title, and also checks to ensure the Block Description is not a duplicate or empty value.
Big thanks go out to chx for assistance.
Robin
Comment #12
(not verified) commentedShouldn't we add a test too to check info unicity ?
Comment #13
chx commentedTo me, this seems like a check for info unicity... not to mention that the UNIQUE(info) DB constraint is not touched.
Comment #14
tostinni commentedUps, sorry for the anonymous and the message, I didn't saw this verification, I was only remembering block.module code.
That's all fine for me.
Comment #15
njivy commented+1 to Robin Monks' patch. When I install Drupal, the first thing I always do is remove the unique key constraint on block titles.
I have not tested the other parts of Monks' patch, though, including the uniqueness constraint on block descriptions.
Comment #16
drummCode looks okay. +1
"You must enter unique data into the block description field." doesn't sound like the friendliest of error messages.
Comment #17
dries commentedI'm OK with this patch but suggest that we look into improving the error message.
Small nit: fix the spacing around '=' in the SQL query that checks for duplicates.
Comment #18
robin monks commentedHere is a new patch with the changes and with the updates.inc file changed to reflect patchs applied after the original patch was made.
Robin
Comment #19
dries commentedCommitted to HEAD. Thanks. The patch did not apply against DRUPAL-4-6. Possibly needs to get backported.
Comment #20
robin monks commentedHere is the patch backported for 4.6.
Robin
MozNetwork.org
Comment #21
robin monks commentedThat patch had a bug in it, new patch attached.
Robin
Comment #22
killes@www.drop.org commentedThe patch got apparently into core. It introduces a problem with php 5.
$edit is not defined as an array if the form is called for the fiest time (default in switch).
This patch fixes this.
Comment #23
Steven commentedSorry for dropping in so late, but I disagree that block descriptions should be filled in.
A while ago I even made a patch to display the block title in the admin list if the description was empty: I never fill in descriptions for my own blocks, I know what they are.
It seems a big usability hit to force people to enter descriptions.
Also, the updates seem to re-add the index on (title) but not on (info). I think the two versions (database.sql and updates.inc) are now out of sync.
Comment #24
RobinMonks commented>Sorry for dropping in so late, but I disagree that block descriptions should be filled in.
>A while ago I even made a patch to display the block title in the admin list if the description was empty: I never >fill in descriptions for my own blocks, I know what they are.
Then please submit it! But from a usibility standpoint, it's better to show a kind error now (when 4.6 is going out the door), then to argue about this and have this bug still in 4.6.
> It seems a big usability hit to force people to enter descriptions.
Point taken, but ATM entering two blank descriptions makes MySQL go mad, and that seems worse to me. We can always remove then need of a unique info later.
>Also, the updates seem to re-add the index on (title) but not on (info). I think the two versions (database.sql >and updates.inc) are now out of sync.
The title was never remove from info if you look closely, and as a matter 'o fact, that index is just left there from redundancy as the module check for dupes on-the-fly.
Robin
Comment #25
Steven commentedThat patch was already committed. So the code I added back then is now redundant as $block['info'] must always be set.
The MySQL problem just seems to be that you're keying on the title or description. Why not have a block id? We do this everywhere else...
Comment #26
robin monks commentedObviously Fixed.
Comment #27
sami_k commentedVery cool! I actually made a feature request for such a feature recently, good to know it's underway.
Comment #28
buddaComment #29
shane commentedWhat happened? I thought that this patch made it into the core code? I'm running 4.6.5 and my block.module still does not allow me to have multiple blank block titles.
This is rather annoying and frustrating, since I *don't* want them. I tried applying the last two patches in this thread, but each of them nuked my system (output a blank white screen each time). So I rolled them back. (yes, I only applied one patch, tested, then rolled back, then tried the other).
Does anyone know the status of why Block Titles are not allowed to be unique in 4.6.5? And if not, is there a working patch floating around out there that does allow it to be unique?
THANKS!
Comment #30
morbus iffDrupal 4.6.1 through 4.6.5 are bugfix and security fixes - there are no new features added. This ability exists only in Drupal 4.7. Until then, look into CSS's "display: none;" on the unique block elements whose titles you wish to hide.
Comment #31
shane commentedThanks Morbus - uh, ayd more depth on that pointer? I'm not sure exactly what you mean by using the CSS "display: none" element...or more precisely, exactly how to implement that to hide the title. I did try and search, but both "display" and "none" are almost noise words. Thanks a ton for the quick reply.
Comment #32
morbus iffTake a look at CSS's display:none. The idea is, if you didn't want to show titles on any block, you could add, via CSS, the following:
.block .title { display: none; }. That *should* keep the code in the actual HTML, but have it be invisible in the actual display.Comment #33
shane commentedAh got it! Which sort of brings up, how does one have titles displayed on some blocks, but not others. I *used* to simply use an HTML comment in the Title, which would be "invisible", since it was an HTML comment entity. However, with the change in 4.6 (somewhere along the line), to not parse the title strings, this breaks any methods of using an HTML comment, or CSS use within the actual title itself to "hide" the title.
At this point, the only thing I can think of is to create a title tag like "hide: TITLE" in the title of only the blocks that I want hidden, then in my theme template use PHP to look for that string "hide:" in the title, and not output the title. Obviously this would only work for custom add blocks. One would have to add additional code to parse for other block titles to hide them. An ugly hack ... but it should work.
Thanks for the follow ups and pointers - I do appreciate it.
Comment #34
morbus iffEach block has their own unique ID. To hide the title of just the login block, use
#block-user-0 .title { display:none; }. View the HTML source of your page to find the IDs of each block.