Steps to reproduce the bug

1. Create a new node and provide a menu link
2. Save
3. Edit the node again
4. The node link itself is listed as a possible Parent item.

Files: 
CommentFileSizeAuthor
#74 drupal.menu-link-self-in-parents.73.patch1.36 KBsun
PASSED: [[SimpleTest]]: [MySQL] 37,281 pass(es).
[ View ]
#71 menu_link_itself_in_parents-955848-70-testonly.patch660 bytesanrikun
FAILED: [[SimpleTest]]: [MySQL] 34,368 pass(es), 1 fail(s), and 0 exception(es).
[ View ]
#71 menu_link_itself_in_parents-955848-70.patch1.4 KBanrikun
PASSED: [[SimpleTest]]: [MySQL] 34,367 pass(es).
[ View ]
#69 menu_link_itself_in_parents-955848-69.patch773 bytesanrikun
PASSED: [[SimpleTest]]: [MySQL] 34,247 pass(es).
[ View ]
#66 missing-argument.patch939 bytescasey
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch missing-argument.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
#59 955848_58.patch6.02 KBchx
PASSED: [[SimpleTest]]: [MySQL] 33,007 pass(es).
[ View ]
#55 menu_link_itself_in_parents-955848-55.patch5.81 KBanrikun
PASSED: [[SimpleTest]]: [MySQL] 32,201 pass(es).
[ View ]
#51 menu_link_itself_in_parents-955848-51.patch5.89 KBanrikun
PASSED: [[SimpleTest]]: [MySQL] 32,198 pass(es).
[ View ]
#41 just_tests.patch2.07 KBcatch
FAILED: [[SimpleTest]]: [MySQL] 30,029 pass(es), 2 fail(s), and 0 exception(es).
[ View ]
#41 menu_link_itself_in_parents-955848-37.patch5.77 KBcatch
PASSED: [[SimpleTest]]: [MySQL] 30,041 pass(es).
[ View ]
#36 menu_link_itself_in_parents-955848-36.patch5.77 KBanrikun
PASSED: [[SimpleTest]]: [MySQL] 29,895 pass(es).
[ View ]
#37 menu_link_itself_in_parents-955848-37.patch5.77 KBanrikun
PASSED: [[SimpleTest]]: [MySQL] 32,025 pass(es).
[ View ]
#20 menu.module.patch3.44 KBanrikun
PASSED: [[SimpleTest]]: [MySQL] 31,543 pass(es).
[ View ]
#14 menu-module-955848.patch3.59 KBKars-T
FAILED: [[SimpleTest]]: [MySQL] 31,258 pass(es), 2 fail(s), and 4,368 exception(es).
[ View ]
#5 menu.module.patch3.32 KBanrikun
PASSED: [[SimpleTest]]: [MySQL] 26,757 pass(es).
[ View ]
#3 menu.module.patch3.23 KBanrikun
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch menu.module_21.patch.
[ View ]
#1 menu.module.patch3.23 KBanrikun
FAILED: [[SimpleTest]]: [MySQL] 26,743 pass(es), 9 fail(s), and 0 exception(es).
[ View ]

Comments

Title:When editing an existing node with a link, the link itself is listed in "Parent item".When editing an existing node with a link, the link itself is listed in "Parent item": menu_parent_options needs some rewriting
Assigned:Unassigned» anrikun
Priority:Normal» Major
Status:Active» Needs review
StatusFileSize
new3.23 KB
FAILED: [[SimpleTest]]: [MySQL] 26,743 pass(es), 9 fail(s), and 0 exception(es).
[ View ]

The attached should fix 2 issues:

1. When editing an existing node with a menu link, parent item options should not list the current link itself nor its descendants.
menu_parent_options function currently only returns options for a given menu item or a given node type but not both.
This is needed, that is why a 3rd optional parameter type has been added to this function.
This way, parent item options do not list the current link itself nor its descendants.

2. 'navigation:0' should not be returned as the default parent when we do not even know if it is present in options!
Instead, the returned default parent is now the first available option.

Status:Needs review» Needs work

The last submitted patch, menu.module.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new3.23 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch menu.module_21.patch.
[ View ]

Ooops, there was a stupid bug, sorry.

Status:Needs review» Needs work

The last submitted patch, menu.module.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new3.32 KB
PASSED: [[SimpleTest]]: [MySQL] 26,757 pass(es).
[ View ]

This time I uploaded the wrong file. I should have slept a little more last night!

Priority:Major» Critical

I'm changing this to critical as selecting the link itself as its parent may break the menu system.

Are you sure that is #550254: Menu links are sometimes not properly re-parented don't fix you issue?

I haven't tried the patch at #550254 yet but it is related to a different issue:
My patch fixes a bug in the Menu module.
The patch at #550254 fixes a bug in includes/menu.inc

Priority:Critical» Major

While this is a bit of a wtf, it doesn't cause any obvious issues as far as I can tell, and hence not critical. iirc Drupal 6 lets you do the same. Downgrading to 'major' since with the new slimmed down menu options, it's a lot more obvious that you can do this than it used to be.

No this is not possible in Drupal 6. Look at modules/menu.module, line 399:

<?php
   
// Generate a list of possible parents (not including this item or descendants).
   
$options = menu_parent_options(menu_get_menus(), $item);
?>

Do you really think it is not critical that a user may set a descendant as a parent?
I do!

No, critical bugs are those that render the system unusable, are a security risk, or cause data loss. This is just an (albeit annoying) bug, see http://drupal.org/node/45111 for the guidelines on issue priority. If you can demonstrate that it will actually break a site or cause irreversible corruption to {menu_links} I'd agree on the critical status, but not before then.

Hi,

I can reproduce the bug and the patch works.

But I have some questions about the code and don't understand it completly. Here a quick question:

+++ modules/menu/menu.module 2010-11-02 09:41:19.921875000 +0100
@@ -333,18 +336,18 @@ function menu_parent_options($menus, $it
-    // If $item is a node type, get all available menus for this type and
-    // prepare a dummy menu item for _menu_parent_depth_limit().

+++ modules/menu/menu.module 2010-11-02 09:41:19.921875000 +0100
@@ -333,18 +336,18 @@ function menu_parent_options($menus, $it
-    // If $item is an array fill it with all menus given to this function.

Why remove the comments?

Powered by Dreditor.

Status:Needs work» Needs review

This is because these comments no longer match what's done by the modified code below them.

Status:Needs review» Needs work
StatusFileSize
new3.59 KB
FAILED: [[SimpleTest]]: [MySQL] 31,258 pass(es), 2 fail(s), and 4,368 exception(es).
[ View ]

Okay sorry for that at a first gaze it seemed so similar.

The problem occurs because only the nodetype is given in original menu.module line 505 and you separate it to an optional parameter. I think this is great but maybe the algorithm could be simplified?

menu_parent_options() is currently used three times. Only the call from menu_form_node_form_alter() uses a string and not an array. So maybe we can make sure $item is always an array. By this there would never be a parameter duplication and the algorithm is easier to read.

Please take a look at my patch that builds on yours. Do you think this is easier?

Maybe even $item could be optional as it can default to

<?php
$link
= array('mlid' => 0);
?>

+++ modules/menu/menu.module 2010-11-02 09:41:19.921875000 +0100
@@ -648,9 +651,9 @@ function menu_form_node_form_alter(&$for
+    $default = reset(array_keys($options));

This gives us the root item of the current menu and is a nice addition imo.

Powered by Dreditor.

Status:Needs review» Needs work

@Kars-T: I'll review your patch ASAP :-)
Why don't you turn this issue into "needs review" again?
This way your patch could be tested.

Status:Needs work» Needs review

Hi,

because I wanted your opinion first. And because the calls for menu_parent_options() could be made similar. So there is still some work left :)

But sure it can't be a bad thing to let the testbot onto it. I am setting this to needs review.

Status:Needs review» Needs work

The last submitted patch, menu-module-955848.patch, failed testing.

Hmm a lot of failures. So my patch seems to be too much of an API change.

I want to help but unfortunately too busy this week :-(
I will review your patch ASAP!

Assigned:anrikun» Unassigned
Status:Needs work» Needs review
StatusFileSize
new3.44 KB
PASSED: [[SimpleTest]]: [MySQL] 31,543 pass(es).
[ View ]

I think like you that as your patch changes how menu_parent_options() is used, it fails testing.
I had kept this in mind when writing my patch: it was made to add features to the original function without breaking the way it was originally used.
So I think it is better to keep my patch.
I have added extra comments to it based on your remarks.

@all: Please review and commit this patch.
I have finished working on this issue.

Status:Needs review» Reviewed & tested by the community

Okay I retested the patch and can confirm that this patch is a solution to the problem. imho it applies to the coding standards and is well commented. So I set this to RTBC.

Thanks for your work anrikun :)

Thank you very much Kars-T :)
By the way, I add a reference to a different but still menu-related issue that needs review too:
#955700: menu/menu.admin.inc lets users add or move node menu links to unavailable menus.

Priority:Major» Normal

I'd like to get some reviews on this from one or more of the menu system maintainers before moving it back to RTBC. But I'm pretty sure coupling menu_parent_options() to the node system is not going to fly.

We also need tests for this to ensure we don't get the bug again.

Finally, I'm not sure why this is "major". It's just a bug.

Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs tests

Priority:Normal» Major
Status:Needs work» Needs review

But I'm pretty sure coupling menu_parent_options() to the node system is not going to fly.

What do you mean? Even current menu_parent_options() makes references to node types.
Why do you revert this from RTBC to "Need works" by the way? Menu system maintainers won't review anything doing so. And adding tests is just an extra task.
I do think it is a major regression bug from D6.

Hi webchick.

I did set this to rtbc because I think it means "reviewed and tested by the community" which I did. As anrikun said I thought now the menu maintainers have to give an opinion as this doesn't mean "ready to be committed". So I'd say it is still rtbc :)

But I don't dare to reset it because mighty webchick said "oh noz" ;)

And about "major": I second that this is a big problem if you could attach a node to itself even if this bug was in D6.

AFAIK this bug was not in D6 (see #10), it is a regression bug.

Ah okay sorry. Was my missunderstanding.

subscribe

subscribe

@jastraat, @zilverdistel:
Did you review the patch?

@anrikun: Apparently not, since I didn't mention it. I'm working on some other issues now.

@anrikun
I just reviewed the patch against the latest -dev and did not get any errors. (And it fixed the problem)

@jastraat: Thanks for your review. I suggest you mark this issue as RTBC.

Status:Needs review» Needs work

+++ modules/menu/menu.module Sat Jan 22 18:34:41 2011
@@ -648,9 +655,10 @@
+  // If the current parent menu item is not present in options, use the first available option as default value.
+  // @todo User should not be allowed to access menu link settings in such a case.

Not to pick nits, but comments should break at 80 characters.

Also:

We also need tests for this to ensure we don't get the bug again.

This means writing a test which verifies

When editing an existing node with a menu link, parent item options should not list the current link itself nor its descendants.

So write a test which creates two nodes 'A' and 'B', assigns menu items to each, with 'B' being the child of 'A' and then verify that you are not allowed to set the parent of 'A' to 'A' or 'B'.

Powered by Dreditor.

Version:7.x-dev» 8.x-dev
Priority:Critical» Major
StatusFileSize
new5.77 KB
PASSED: [[SimpleTest]]: [MySQL] 29,895 pass(es).
[ View ]

Here is an updated patch. Changes are:
- Applies to 8.x
- Comments break at col 80
- Added tests (added tests fail without the patch, as expected)

Status:Needs work» Needs review
Issue tags:-Needs tests
StatusFileSize
new5.77 KB
PASSED: [[SimpleTest]]: [MySQL] 32,025 pass(es).
[ View ]

The same patch should apply to 7.x too:

Priority:Major» Critical

Changing this to "critical":
If the user by mistake sets the node itself as its parent, then reverts this, the menu item is still displayed as having children ({menu_links}.has_children = 1 when it should be 0).
So data integrity is broken.
The only workaround to clean data back is to attach a new menu item as a child of the broken one, then to remove this new item.

Priority:Major» Critical
Status:Needs review» Needs work
Issue tags:+needs backport to D7

@webchick, that function already takes $item as either an menu item or a node type string, so this patch doesn't couple the menu system/module to node any more than it already is.

This patch really, really needs more docs about why we're adding a $type parameter to menu_get_options() when that overloading of the arguments is already in place though. I can't see why exactly it's needed to fix this particular bug at the moment (is it standing in for $is_a_node?), nor when it's appropriate to to pass one vs. the other. This might be a case where we want a _menu_get_options() with the extra param so it hides the API addition from everyone on 7.x (and the bizarre almost-double parameter from 8.x for that matter).

Also this needs to go into 8.x first and be backported, changing version back again and adding tag.

Status:Needs work» Needs review

@catch: this is explained at #1:

menu_parent_options function currently only returns options for a given menu item or a given node type but not both.

Also at #20:

[...] patch: it was made to add features to the original function without breaking the way it was originally used.

To summarize:
1. when $item is a node type, the function returns the menu options for this node type (original behaviour; $type is ignored).
2. when $item is a menu item (and $type is left empty), the function returns the menu options for this menu item (original behaviour).
3. when $item is a menu item and $type is given, the functions returns the menu options for these menu item and node type (behaviour added by the patch).

The way the original function works is absolutely unchanged. That's why the patch successfully passes the tests. Only an extra and optional parameter is added.
So we are sure committing it won't break current 7.x API.

StatusFileSize
new5.77 KB
PASSED: [[SimpleTest]]: [MySQL] 30,041 pass(es).
[ View ]
new2.07 KB
FAILED: [[SimpleTest]]: [MySQL] 30,029 pass(es), 2 fail(s), and 0 exception(es).
[ View ]

Uploading just the full tests and the full patch again to ensure the tests catch the bug, looks like they will but it's nice to see the red and green together ;)

Assuming that runs OK I'll mark this back to RTBC.

Wow, I didn't know it was possible to run 2 tests at once like that :-0
Thanks catch!

Priority:Critical» Major
Status:Needs review» Reviewed & tested by the community
Issue tags:+Usability

I read through this again and it looks like the minimal possible change to get this fixed.

webchick had two concerns when she marked this back to CNW:

1. That it ties menu_get_options() to node module, it's already tied to node module so there's no added coupling.

2. No tests - the new tests look good to me and catch the bug.

However I'm marking this back down from critical again, there is data inconsistency here but it is extremely minor and depends on the edge case of the user actually choosing the node as a parent link for itself. This is a bizarre usability wtf though so adding that tag.

Status:Reviewed & tested by the community» Needs review

You forgot 3) sign-off from a menu system maintainer. :) This code looks like it's introducing a bunch of ugliness to work around a relatively minor edge case, and I want to make sure it gets sanity-checked by someone who's expected to maintain this code going forward.

Webchick, when you make RTBC status reliant on sign-off from specific subsystem maintainers, you reduce the pool of reviewers down to almost zero (in this case 2, one of whom is sick this week).

Should I stop bothering to mark major bug reports RTBC for subsystems that I don't maintain then? That would make my life easier.

I try to only do this in cases where I'm dubious of it passing the muster. If it's a no-brainer fix I've no problem committing it. Experience has taught me though that subsystem maintainers get exceptionally grumpy when things that don't pass the sniff test are committed without their consultation.

So, no. You shouldn't stop marking bug reports RTBC. But neither should we ignore the contents of MAINTAINERS.txt, especially on "borderline" issues like this.

Status:Needs review» Needs work

Good grief, we have The menu item or the node type for which to generate a list of parents. in HEAD? Did I write that? It's ugly enough for that...

Anyways. Patch looks solid indeed with a few minor nitpicks

+    // Use it as $type and prepare a dummy menu item.
-    // prepare a dummy menu item for _menu_parent_depth_limit().

You already had a comment which explains what dummy is for, why are you removing that?

+  $options = menu_parent_options(menu_get_menus(), ($link['mlid'] ? $link : $type), $type); This really needs a comment.

+    $default = reset(array_keys($options)); this is an E_STRICT error, reset needs a variable. Yes. I hate E_STRICT too.

This code looks like it's introducing a bunch of ugliness to work around a relatively minor edge case

@webchick: do you think that it is the correct way to treat community? Read http://drupal.org/dcoc
I am trying to help to fix a bug and spent time to provide everything that was asked: clean code, tests, etc.
But it seems that you keep ignoring all the explanations that I have posted in this issue so far. This bug is not minor: it was not in D6 and can lead to data inconsistency.

Well, if you think that non-maintainer people from the community should not take part to core bug fixes, then it should be stated somewhere: RTBC means "Reviewed and tested by the Community", not "the Maintainers".

(Sorry for my English)

This make me cry a bit:

+  if (!is_array($item)) {
+    // If $item is not an array then it is a node type.
+    // Use it as $type and prepare a dummy menu item.

Not the patch so much as how something this ugly got in core.

anrikun: I'm really sorry! That comment wasn't intended to be at all an insult to you. It was in reference to the solution that had to be engineered in order to work around this problem—having to create a dummy menu item, adding a node type as a parameter to a function that is general-purpose to menu module and not at all related to nodes, etc. I guess a better word would've been "complicated" or so.

I do understand the bug, and how it can be triggered, and how it will lead to data corruption when it does. I just wanted to make sure that this solution was supported by one of the folks who knows this code inside and out, since I don't, and it raised by "spidey sense" for things that might not quite be core-worthy, yet.

However, regarding RTBC status, that's a recommendation to the maintainers to commit something. It's not a carte-blanche "ok, this will now get in. now hurry up and press the button." It's in fact my job as core maintainer to say "Hm. But what about this?" or "Are we sure that's the best way to fix this?" So that's what I will continue to do.

But don't think that doesn't mean that I don't appreciate your contributions, and your willingness to dig in and fix this. I just want to make sure the fix stands the test of time, and the best way to do that is to consult with experts when in doubt.

Status:Needs work» Needs review
StatusFileSize
new5.89 KB
PASSED: [[SimpleTest]]: [MySQL] 32,198 pass(es).
[ View ]

@webchick: Thank you for your explanations.
@chx: about the removed comment, see #13.

Here is an updated patch. Changes:
- Changed

<?php
// Use it as $type and prepare a dummy menu item
?>
into
<?php
// Use it as $type and prepare a dummy menu item for _menu_get_options().
?>

- Changed
<?php
$options
= menu_parent_options(menu_get_menus(), ($link['mlid'] ? $link : $type), $type);
?>

into some clearer code.
- Fixed the E_STRICT issue.

@webchick

having to create a dummy menu item, adding a node type as a parameter to a function that is general-purpose to menu module and not at all related to nodes, etc.

This was not a correct statement in January, and it remains wrong now.

anrikun pointed this out in #25, I corrected it in #39, then again #43 so I'm not sure why you are repeating it again.

http://api.drupal.org/api/drupal/modules--menu--menu.module/function/men...

$item The menu item or the node type for which to generate a list of parents. If $item['mlid'] == 0 then the complete tree is returned.

The wtf is already there in Drupal 7, multiple people on this issue have pointed it out, can we stop blaming it on this patch?

For the record, I wanted to see where this horrible code was added, and git blame + git log showed it was in #351249: Finer control over the Parent Menu select box, committed October 2009.

No-one (including me who RTBC'ed one of the versions of the patches there, nor Dries who did in-depth reviews himself then committed straight from CNR) caught that particular bit of ugliness, I think at least in part because it had been identified as a critical UX issue, and the last flurry of work was close to the final week before the 'UX freeze' for Drupal 7 during code slush, or that is how I'm rationalizing it today (other than 2009 and 2011 catch having different review criteria).

I will fully take my fair share of the blame for letting the ugly slip through in 2009, if you will accept that we are not letting it slip through now in 2011 - the patch here only continues the process of cleaning up the mess that patch added.

Let's quote from that issue:

OK, this looks good enough. Committed to CVS HEAD. We can refine in follow-ups as necessary.

This is one followup, #761648: Menu D6->D7 upgrade doesn't maintain node-menu configuration (f.k.a.: Menu items disappear after upgrade or manual menu entry) was another (and only got committed this month). It is a shame they are happening two years too late and within the constraints of an already released core version, but that is why we need to clear through the backlog of broken and buggy crap before adding new stuff to D8 that we'll end up fixing in 2014 or 2015.

We can open a new issue for Drupal 8 only to roll back the whole thing maybe?

Status:Needs review» Needs work

I am sorry for putting this back to CNW -- and for not explaining this one better -- but $options = menu_parent_options(menu_get_menus(), ($link['mlid'] ? $link : $type), $type); the issue is not with the ?: operator. I asked for a comment because the WTF here is ($link['mlid'] ? $link : $type). Questions immediately jumping at me: so $link['mlid'] is 0 in this case? But mlid is a serial, how can a serial be 0? Is this coming from the dummy? This is what needs explaining in a comment. You can keep this in a ?: again, that's not the problem.

Status:Needs review» Needs work

@chx: does it really need any extra comment? I mean, in menu.module it seems to be assumed that new (not saved yet) menu items have 0 as 'mlid'.

See:

<?php
/**
* Implements hook_node_prepare().
*/
function menu_node_prepare($node) {
  if (empty(
$node->menu)) {
   
// [...]
    // Set default values.
   
$node->menu = $item + array(
     
'link_title' => '',
     
'mlid' => 0, // <-- It's here!
     
'plid' => 0,
     
'menu_name' => $menu_name,
     
'weight' => 0,
     
'options' => array(),
     
'module' => 'menu',
     
'expanded' => 0,
     
'hidden' => 0,
     
'has_children' => 0,
     
'customized' => 0,
    );
  }
 
// [...]
}
?>

There is no comment about the fact that $item['mlid'] = 0 when it is a new item anywhere in the file (just look for the 'mlid' pattern occurrences in the file and you will see). So why add one especially for this case?

Status:Needs work» Needs review
StatusFileSize
new5.81 KB
PASSED: [[SimpleTest]]: [MySQL] 32,201 pass(es).
[ View ]

I have put $options = menu_parent_options(menu_get_menus(), ($link['mlid'] ? $link : $type), $type); back.

Status:Needs work» Needs review

Drupal 7.7 is out, but...
Too bad that this is not committed yet :-(
(catch had RTBCed this patch on June 10)

Tagging issues not yet using summary template.

@anrikun Can you just add a comment? Sure the other place doesn't do it, but if it adds clarity we should pursue it. Clarity over consistency any day.

StatusFileSize
new6.02 KB
PASSED: [[SimpleTest]]: [MySQL] 33,007 pass(es).
[ View ]

I have written a comment and nuked a superflous argument. I still think calling such a goofy function (oh god, did I write that one??) with such weird arguments needed a comment.

Status:Needs review» Reviewed & tested by the community

Seconded for RTBC. Tested patch in #59 against D7, it solves the original issue when using menu_admin_per_menu.

subscribe

#59: 955848_58.patch queued for re-testing.

Status:Reviewed & tested by the community» Fixed

Looks good. Committed to 7.x and 8.x.

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Status:Closed (fixed)» Needs review
StatusFileSize
new939 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch missing-argument.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

The "superflous" argument is required after all. If you edit an exiting node with a menu link, the third argument is used. If it is not passed the user may choose a parent link from all menus, not only the configured ones.

Status:Needs review» Needs work

The last submitted patch, missing-argument.patch, failed testing.

Priority:Major» Critical
Status:Needs work» Needs review

Critical as it introduced a regession for the 7.10 release.

StatusFileSize
new773 bytes
PASSED: [[SimpleTest]]: [MySQL] 34,247 pass(es).
[ View ]

An updated patch below: basically, it's what was supposed to be committed at #55 + comments added by chx at #59.

Status:Needs review» Needs work

Hmm, reverting to "needs work".
Actually we need to add a new test so that case at #66 is properly tested too.

Status:Needs work» Needs review
StatusFileSize
new1.4 KB
PASSED: [[SimpleTest]]: [MySQL] 34,367 pass(es).
[ View ]
new660 bytes
FAILED: [[SimpleTest]]: [MySQL] 34,368 pass(es), 1 fail(s), and 0 exception(es).
[ View ]

I have updated the patch adding the missing test.
First patch below only contains the test and should fail nicely.
The second one is the final patch (fix+test) and it should pass the test.

Status:Needs review» Reviewed & tested by the community

Looks good, so I'm marking RTBC. Will commit in a few days if no objections.

Version:8.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)

Committed/pushed to 8.x.

Moving to 7.x for backport.

Status:Patch (to be ported)» Needs review
StatusFileSize
new1.36 KB
PASSED: [[SimpleTest]]: [MySQL] 37,281 pass(es).
[ View ]

Re-rolled for D7.

Status:Needs review» Reviewed & tested by the community

Straight re-roll, back to RTBC.

Please commit to D7 too so that I can submit a new patch that moves function assertNoOption() to simpletest drupal_web_test_case.php according to the todo.

Actually, should this new patch be submitted as a separate issue or as a follow-up of this one?
And should it be marked as a simpletest.module issue?
Don't really know what's the right way to do in such a "crossed" case...

That should be a separate issue, either component is fine (or 'base system' if it really does go across everything).

Priority:Critical» Normal
Status:Reviewed & tested by the community» Fixed

Committed and pushed to 7.x. Thanks!

Status:Fixed» Closed (fixed)
Issue tags:-Usability, -Needs issue summary update, -needs backport to D7

Automatically closed -- issue fixed for 2 weeks with no activity.