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.
This issue is the continuation of #955848: When editing an existing node with a link, the link itself is listed in "Parent item": menu_parent_options needs some rewriting
Provided patch moves function assertNoOption()
to simpletest drupal_web_test_case.php
according to the todo.
Comment | File | Size | Author |
---|---|---|---|
#10 | move_assertNoOption-1413888-10.patch | 2.95 KB | anrikun |
#6 | drupal8.assert-option.6.patch | 2.98 KB | sun |
#3 | move_assertNoOption-1413888-3.patch | 2.98 KB | anrikun |
#1 | move_assertNoOption-1413888-1.patch | 2.35 KB | anrikun |
Comments
Comment #1
anrikun CreditAttribution: anrikun commentedBelow is the patch:
Comment #2
sunIf we introduce an assertNoOption(), then we should also introduce a assertOption().
Let's also add data type-hinting to the phpDoc @param and @return, and add a blank phpDoc line between @param and @return.
Comment #3
anrikun CreditAttribution: anrikun commentedThere you are: added function
assertOption()
.But I haven't touched the phpDoc part, because I've just stuck to the way it is written for other functions in the same file. I mean, if we change phpDoc for these 2 functions, we should also change it for all the others functions too. But I guess it is a separate task.
Comment #4
sunThanks!
oink! Actually, these // need to be turned into a single /. The double // opens up a new global selector.
Apparently, that only seems to be the case in some versions of PHP. In some others, the consecutive // is an alias of /descendant::, but not in all. Hence, the //option matches any option element on the page, not necessarily a child of //select.
(At least I was able to confirm that latter wonky behavior on Windows at some point.)
Comment #5
anrikun CreditAttribution: anrikun commentedAgain, all other XPaths that select descendants are written this way in
drupal_web_test_case.php
.My patch here is only supposed to address Move function assertNoOption() from menu.test to simpletest drupal_web_test_case.php.
Do you ask me to fix the other functions (and phpDocs) of
drupal_web_test_case.php
too?I'm a bit lost here.
Comment #6
sunSorry, where did I ask to fix all?
Comment #7
anrikun CreditAttribution: anrikun commentedBecause you had reverted status to needs work, I thought that I needed to do something :-)
Anyway, I'm not sure that replacing // (descendant) by / (child) is correct because options might be nested in
optgroup
tags.As this looks like a bug of some PHP version, should we really address this here?
Comment #8
sun#3 (not latest patch) is RTBC.
That is, as long as I don't get negative results for https://twitter.com/tha_sun/statuses/161967541944721412
Comment #9
Dries CreditAttribution: Dries commentedCommitted to 8.x.
Would be good to follow-up with a patch that actually uses the new API. Could make for a good round of clean-up.
Comment #10
anrikun CreditAttribution: anrikun commentedRerolled patch at #3 for 7.x.
Comment #11
anrikun CreditAttribution: anrikun commentedMaybe this issue requires a different Status or a tag? "Backport to D7"?
Comment #12
sunComment #13
sun#10: move_assertNoOption-1413888-10.patch queued for re-testing.