Placing a block in a region via select dropdown moves it to the top of the region, but it will show at the bottom

Confirmed. In blocks ui, if you use the select list to move blocks into a region, it is shown inserted on top but after saving and viewing in the front-end, the block is shown last in the region. Drag-dropping works, happens only when using the select list.

Honoring the insert-on-top make most sense to me because it will show the region name directly above it, giving you direct confirmation you put it in the right region.

#1308982: wrong sorting when assigning block to a region

Gave a try at writing a patch for this :-)

forgot the status

The patch looks good codewise. Note that the Drupal 8.x patch will need to be rerolled, because the core directory structure for Drupal 8 has now changed. (For more information, see #22336: Move all core Drupal files under a /core folder to improve usability and upgrades). When the patch has been rerolled, please set the issue back to "Needs Review."

Tagging as novice for the task of rerolling the Drupal 8.x patch.

If you need help rerolling this patch, you can come to core office hours or ask in #drupal-gitsupport on IRC.

Also, it would be good to get a couple users testing this manually in different browsers and confirming the issue is resolved as expected.

will reroll patch asap :-)

rerolled patch

Works fine on webkit + firefox. Guess this is the best way in the patch, since the DOM of blocks page is tricky.

Confirming this fixes the problem for me too.

Hurrah! This has been a real niggle on D7 - tagging for backport.

OK patches like this make me wish we had automated JavaScript testing, but given this has been manually tested and it's a small change, committed/pushed to 8.x.

Will need a quick re-roll for 7.x.

rerolled patch for drupal 7

The patch makes the same modifications and applies cleanly. I tested with a fresh version of Drupal 7 and after applying the patch blocks moved using the selection list now appear at the bottom of the selected region as they should.

This bug is also in D6. Let's fix it while here :) one problem is nextUntil() is only in jQuery >= 1.4, and D6 core is using 1.2.6. So better finding another way to solve it.

It is a bit of a workaround, but this patch seems to work:

err, about the status of this issue, should this be RTBC because of the D7 patch or needs review for the D6 patch, I'll put it back to RTBC so we can get the D7 patch in first?

patch failed to apply because it tried to apply to D7 branch. Re-test once the D7 patch gets in...

testbot failed.

ahh #11 is D7

failed patch is D6

+++ b/modules/block/block.js
@@ -45,11 +45,15 @@ Drupal.behaviors.blockDrag = function(context) {
-      // Find the correct region and insert the row as the first in the region.
+      // Find the correct region and insert the row as the last in the region.

Is this change in behavior required for fixing the bug? can't we fix it without making changing the default order from first to last?

Anyhow patch in #14 tested manually and working, but note the point I mentioned the line above.

P.S. put a -D6 at the end of the patch name to stop testbot from testing it.

Powered by Dreditor.

That is the exact bug we're trying to fix?

I can't edit the patch name anymore now but I will remember this for the future :-)

The actual bug is, when you select blocks they got sorted on top, when you save they got missed up. The order top-first (or as you edited it bottom-first) is not as important as when saving I get different sort than the one I saw when I selected the blocks from their region-selectbox.

PHP coding is correct, JS errors. mark #11 RTBC.

Committed and pushed to 7.x. Thanks!

Marking down to 6.x.

#14: drupal6-1039666-14.patch queued for re-testing.

Could I bother others to manually retest this patch for D7.x again. The patch in #11 seems to have broken the blocks admin screen.

A fresh install of D7-dev rolled 12/25/2011, disabled blocks are no longer able to be set to any empty region. They get pushed to the bottom of the disabled section. Upon trying to set them again, they disappear. I have narrowed it down to this change (the change in #11). When I overwrite the -dev version of block.js with a version of the file from 7.10 the block screen works again albeit with the original issue the patch was trying to fix.

This is occurring in at least Firefox8 and IE8. I've yet to test other browsers.

Steps to reproduce:
go to administer -> structure -> blocks
try setting a disabled block to an empty region
the block gets put at the bottom of the disabled blocks list
try setting it to an empty region again
block disappears from block administration screen

remarking as active.

erp and shifting back to 7.x to get eyes back on it.

I just checked out a fresh copy of D7 via Git.

I am able to successfully set a block's region by using either the draggable handle or the selection list. When using the list to enable a block, the block's table row is correctly moved to the bottom of the selected region and when using the list to disable a block, the block's table row is correctly positioned at the bottom of the disabled region.

Are you still having this problem with a fresh checkout of D7 and can you post the steps to reproduce? I can help test.

I couldn't reproduce either.

Failed assign to an empty block region (No blocks in this region)

yes issue is still reproduceable in the 7.x-dev from 12/28

droplet is correct. The issue isn't with blocks which are being assigned to regions which already have blocks in them. The issue is when a block is being assigned to a region that doesn't yet have any blocks assigned to it.

steps to reproduce.
in the disabled blocks list set a block using the select list to a region which does not have a block in it.
The block is sent to the bottom of the disabled blocks list.

the expected behavior would be that the block is put in the region the was selected.

Cannot reproduce in 7.x dev(12/15) or drupal 8.x dev. Can you give more info about environment and block/region?

Did a clean install of 7.x dev(12/28). Changed nothing and went straight to admin/structure/blocks and was able to reproduce. Not only that, can't move a block currently assigned to a region to a new(empty) region. Can move a block currently assigned to a region to a region that has blocks assigned to it. Something definitely wrong. Running on OS X using sqllite for database and bartik for theme.

Wow, thanks for testing and catching this!!

Ok, reverted these commits:

git revert 67b7ac08eb8a; git commit; git push origin 7.x
git revert b39fe6dbb7; git commit; git push origin 8.x

7.x-dev and 8.x-dev should be back working again.

Heh, and I had made a video to convey what I couldn't seem to in text! FIGURES!

I'm attaching the link to the vid anyway for those who couldn't reproduce or those who land here and don't understand what was happening

http://youtu.be/vL_RFpFoVZI

Built 7.x from git and tested. Can assign blocks to empty regions with no problems. So issue needs work.

fixes and cleanup.

Each block allowed assign to one region only. I see no reason not to select it directly.

// Manually update weights and restripe.
I do not know why we add these codes. tabledrag.js handle these changes.

nextUntil in v1.7 is different:
http://api.jquery.com/nextUntil/

+++ b/core/modules/block/block.jsundefined
@@ -111,30 +111,14 @@ Drupal.behaviors.blockDrag = {
+      $(this).change(function () {

event never used.

+++ b/core/modules/block/block.jsundefined
@@ -111,30 +111,14 @@ Drupal.behaviors.blockDrag = {
+        var row = $(this).parents('tr');

it's always ONE selected row

+++ b/core/modules/block/block.jsundefined
@@ -111,30 +111,14 @@ Drupal.behaviors.blockDrag = {
-        $('tr.region-message', table).each(function () {
-          if ($(this).is('.region-' + select[0].value + '-message')) {

it can be selected directly.

+++ b/core/modules/block/block.jsundefined
@@ -111,30 +111,14 @@ Drupal.behaviors.blockDrag = {
-            // Add the new row and remove the old one.
-            $(this).after(row);
-            // Manually update weights and restripe.
-            tableDrag.updateFields(row.get(0));
-            tableDrag.rowObject.changed = true;
-            if (tableDrag.oldRowElement) {
-              $(tableDrag.oldRowElement).removeClass('drag-previous');
-            }
-            tableDrag.oldRowElement = row.get(0);
-            tableDrag.restripeTable();
-            tableDrag.rowObject.markChanged();
-            tableDrag.oldRowElement = row;
-            $(row).addClass('drag-previous');

redundant codes.

0 days to next Drupal core point release.

reroll

Tested and did not appear to work in 8. Steps:

• I applied patch from 41.
• I cleared the cache
• Using the dropdown, I added Syndicate to Sidebar first. It appeared in the list at the top of that region
• I hit Save blocks
• Syndicate now appears as the second item in the Sidebar first region

As an aside, I was not able to replicate #37.

My first patch review so feel free to comment on ways to improve this report.

Removed Needs manual testing tag since it needs a rework. Sorry for the extra post but did not see a way to edit previous one while removing tag.

Thanks @caelon.

are you sure cleanly apply the patch ? can you try once more, thanks.

Patch doesn't apply anymore due to some small changes to jQuery selectors.

I updated #41 to work with the changes and have the following behaviour on a fresh D8 install.

Moving to already populated region:
1. Move Syndicate to bottom of Sidebar first (4/4) or choose Sidebar first in drop down
2. Syndicate is at bottom of list.
3. Save Blocks and Syndicate is still at bottom of list
4. Check front page and Syndicate is at the bottom!

Moving to empty region.
1. Move Syndicate to empty region
2. Syndicate appears there.
3. Save Blocks and Syndicate is there.

Here is the patch I used for this round of testing. It should apply to the latest D8.

Sorry ??

what's the different between #44 & #45 ?

Line 10
-- $('tr.region-message', table).each(function () {
+- table.find('tr.region-message').each(function () {

Line 24
-- $(row).addClass('drag-previous');
+- row.addClass('drag-previous');

#44 wouldn't apply to the latest D8.

Thanks. can you patch Line 10 to latest version.

Line 10 should be at the latest version now.

This patch should be ready to go.

#45: 1039666-block-admin-page-45.patch queued for re-testing.

Thanks,

after JS clean up, following line

+++ b/core/modules/block/block.jsundefined
@@ -119,24 +119,8 @@ Drupal.behaviors.blockDrag = {
+        $('.region-' + select[0].value + '-message', table).nextUntil($('.region-message')).last().before(row);

should be:

      table.find('.region-' + select[0].value + '-message').nextUntil($('.region-message')).last().before(row);

There were a few redundant things.

here it is a little cleaned up. RTBC otherwise, works fine for me, please confirm.

Works as expected for me. I did find a new problem on a clean installation if one uses the drop down to move a block to a pre-populated region such as Sidebar First.

Upon selecting Sidebar first with the dropdown Syndicate moves to the bottom of the Sidebar first region (fourth position). However, after saving Syndicate appears to be third (Search form, Navigation, Syndicate, User login) this is because the last three blocks all have a weight of 0 while Search form has a weight of -1 so the last three blocks are sorted alphabetically. Weights are (-1, 0, 0, 0) and stay at 0 until one uses the handle to move a block into the region.

I think that should probably be a separate issue though as it is related to the blocks default weightings not js.

Therefore this is RTBC!

Thanks! Committed/pushed to 8.x, moving to 7.x for backport.

This should do it.

Tested with Chrome and Firefox and the behaviour is working as it should be.

I confirmed the backport is straight forward (or backward :D) related to the original 8.x patch and that it indeed works in Chrome and Internet Explorer. Thank you @Kevin Morse!

Wow, that's quite the clean-up. Thanks.

Committed and pushed to 7.x.

Fixing tags per http://drupal.org/node/1517250

I think this is a continuation of this issue, of the last patch at least.

Found bug, when there is no block in the last region (that would be the Disabled region), you can't move a block to that region using the select drop-down.

Steps to reproduce

1. Move all blocks from the last region (Disabled) to other regions.
2. Move one block back to the last region using the region select drop-down.

Other notes

Also other findings:

• tableDrag.row() expects first argument to be DOM element instead of jQuery object, I think.
• checkEmptyRegions() expects the second argument to be an instance of tableDrag.row().
• there are lots of $(this) that could be optimized as $this = $(this); I think, but I didn't dare to make the changes (maybe jQuery has a cache system, or the call isn't expensive, dunno).

I think a new issue should be created. See #2177335: Selecting 'Disabled' does not move the block to the disabled region when there are no disabled blocks.

Started from #61 it seems like a new bug. This isn't a duplicated issue. I fixed the status.