Increase Block Title Length

+1
This looks to be an easy one. As it is, I can't manually insert a long link in the title field. It's too short to do anything useful!

Seems to be 64 characters. In Views, a block title can be 128 characters. Views makes Drupal better(TM).

Here's the patch to change the max-length from 64 to 128. This changes both the DB schema (clean install required) and the form field.
Currently both of these are hard-coded into the respective two files.
I would, however, recommend getting the max-length for the form field straight from the DB schema.
Unfortunately I am not aware of any quick DB-independent ways to get the schema using the available database methods. (Maybe someone else could help out here).
Check the patch file for more info.

Needs to go into Drupal 8.x first.

You should set the status to "needs review" when you provide a patch.

They are a reason to use 128 instead of 255? They is no index on this field so they should be no performance impact.

A patch for D8 than use 255 a limit and the textfield size has 60 chars (like node title).

Needs a hook_update_N() for the block module to handle the schema change when updating an existing install.

Ok now with a hook_update_N but I'm note sure is the right way...

+++ b/modules/block/block.installundefined
@@ -185,3 +185,19 @@ function block_install() {
+ */
+function views_update_7001() {
+  db_change_field('block', 'title', 'title', array(

I think there should be block in stead of views. I also think there are other block_update_N functions in drupal 7? Or not? Your number should be one more than the highest number (if I'm correct)

+++ b/modules/block/block.installundefined
@@ -185,3 +185,19 @@ function block_install() {
+}
\ No newline at end of file

Needs a newline

Powered by Dreditor.

Thanks for your review.

This is actually the first block_update_N for the block module.

- hook name: done
- new line: done

There are some problems with your hook_update_N implementation. Per http://api.drupal.org/api/drupal/includes--database--database.inc/functi..., you need to drop all of the keys and indices and recreate them after you alter the field.

Also, I'm not sure that you need to specify all of the field properties. Just set the length.

Also, according to http://api.drupal.org/api/drupal/modules--block--block.install/7, there are already hook_update_n implementations. Somebody else will have to chime in on this - not sure how to handle it.

Thanks for your review :).

The field has no key or index so I suppose I don't need to do anything.

It's a patch for D8 so I should write my hook like block_update_8000 (I write for D8, but I still think D7, arrgh).

#12: block_title_length-466576-12.patch queued for re-testing.

Hop it's all right.

The patch looks good! This could use an upgrade path test.

Here's an attempt for the upgrade path test.

Also, the patch from #19 is now out of date and needs to use block_update_8003.

AND I would love to see this test in the new PSR-0 format.

Here's the fix and tests.

+++ b/core/modules/block/block.admin.inc
@@ -285,7 +285,8 @@ function block_admin_configure($form, &$form_state, $module, $delta) {
+    '#size'=> 60,

Although my personal preference is not to widen the field's width here, I don't what to hold this issue up because of that.

I just want to call out that we are in code slush here and this part of the patch might be rolled back in the future once the UX folks have a pass on the UI.

RTBC

Actually, the field width shouldn't be changed here. A size of 255 would be very very large.

Sorry? The width isn't being set to 255, just the allowed length of the field.
That said, that #size line is missing a space between ' and =>, so needs work for that.

Whoops. Read it wrong. Thought it was setting #size to 255 for a second.

But yeah. Coding standards and stuff.

/me goes to bed.

the size IS being changed to 60 though. Adding the novice tag for this standards compliance fix.

Back to RTBC per #24, only change was the missing space.

Seconded - looks very solid to me.

I check the test part of the patch, look nice and hard to break for me.

This seems reasonable to me for 8.x, and we're below thresholds atm. However, the patch no longer applies.

However, for Drupal 7, David has requested help on resolving release blockers, so I don't feel comfortable committing feature patches to 7.x until that happens.

This is a re-roll for latest drupal core.

Ups, I included the UpgradeTest file in the wrong folder.

I'm trying to test this on my local machine, but it's the first time I do this and I'm not sure if I'm doing things right. I have latest drupal 8. I created a new site with it. Enabled the testing module and tried to go to admin/config/development/testing but got an error like:

PHP Fatal error: Class 'Drupal\system\Tests\Upgrade\UpgradePathTestCase' not found in /home/jackbravo/work/drupal-sites/git-drupal2/core/modules/system/lib/Drupal/system/Tests/Upgrade/BlockUpgradePathTest.php on line 15

Ok, had a few typos on the class names. Hope this fixes it.

Changing status.

This issue has a duplicate, #549248: Block module and System module allow for different block title lengths.

This seems to be working.

Leaving at RTBC, but I cleaned up a few code style issues.

Leaving at RTBC, but I cleaned up a few code style issues.

Thank you, #1779638: Unexplained test failure in LocaleFileImportStatus->testBulkImportUpdateExisting()

This seems reasonable. Committed/pushed to 8.x.

Not sure about 7.x backport but that's up to David.

If this gets backported, after it's committed to 7.x it'll need to be bumped back to critical and 8.x to remove the update from 8.x. This particular one doesn't do any harm but there's no point having it in either.

TODO: tests for block title max length.

patch block-466576-46-d7.patch it's working fine. on D7.

(none)

Looks good. Applied upgrade to D7 site. Worked nicely.

Random test failure.

Committed to 7.x - thanks!

If this gets backported, after it's committed to 7.x it'll need to be bumped back to critical and 8.x to remove the update from 8.x. This particular one doesn't do any harm but there's no point having it in either.

Forgot to mention that this is not relevant anymore (the update function is not in Drupal 8) so no followup to do here.

A weird failure in the update hook added by this issue has been reported as #2378561: block_update_7009 fails with Warning: 1265 Data truncated for column 'title' at row 75