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.
Currently you can configure page displays to provide itself not only as local task/local action but also
as contextual link.
As #2040379: Define contextual entity operations through annotation; remove the context property from hook_menu items might kill the functionality let's ensure that it is not just ignored.
People might interpretate this patch as a kind of asshole patch (as it makes refactoring difficult), but I just care about hidden and potential regressions.
Comment | File | Size | Author |
---|---|---|---|
#11 | vdc-2051917-11.patch | 5.85 KB | dawehner |
#9 | vdc-2051917-9.patch | 5.93 KB | dawehner |
#9 | interdiff.txt | 2.75 KB | dawehner |
#6 | vdc-2051917-6.patch | 4.48 KB | dawehner |
#4 | vdc-2051917-4.patch | 4.66 KB | dawehner |
Comments
Comment #1
dawehnerComment #3
damiankloip CreditAttribution: damiankloip commentedI don't think this is an 'asshole patch' :) We have been screwed many times by regressions, this is fair enough imo.
Space between
Weird place for this comment, use '//' or we could make it a property or somthing and document it there?
I like to switch these params round, I find it easier to read - that' just preference though. I think unit tests in phpunit may want them this way?
Comment #4
dawehnerThat is a phpstorm thing to be able to document the type of variable, which is just a good idea in general, I think.
I don't really know why this failing, as it works fine locally.
Comment #6
dawehnermhh
Comment #7
dawehner.
Comment #9
dawehnerSeriously testing contextual links is hell.
Comment #10
damiankloip CreditAttribution: damiankloip commentedMaybe 'Tests views contextual links on nodes' instead?
Tests.. sorry
We could still use assertlink() and friends here? I guess you want the count first, so might as well use the xpath result you have?
New label please :)
Comment #11
dawehnerThanks for the review!
Comment #12
tim.plunkettWho did this to contextual links? Ouch.
The ugliness of this code is not @dawehner's fault.
Test looks great.
Comment #13
dawehner#11: vdc-2051917-11.patch queued for re-testing.
Comment #14
Wim Leers#12: you're probably referring to the fact that you need HTTP requests for contextual links to show up; I did that, but it was necessary to fix the render cache, see #914382: Contextual links incompatible with render cache.
If you're referring to test ugliness — then I'm happy to point you to #2074037: Add drupalPostUrl() — drupalPost()/drupalPostAjax() are for forms only, D8 JS performs non-form HTTP requests which fixes that :) mondrake just +1'd that issue and tagged it with VDC because you apparently encountered the same problem in Views.
Comment #15
dawehner#11: vdc-2051917-11.patch queued for re-testing.
Comment #16
webchickYay, more tests!
It seems like this could benefit from #2074037: Add drupalPostUrl() — drupalPost()/drupalPostAjax() are for forms only, D8 JS performs non-form HTTP requests once that's in, but that's still under discussion, so for now, curlExec() it is, I suppose.
Committed and pushed to 8.x. Thanks!