Closed (fixed)
Project:
Drupal core
Version:
8.0.x-dev
Component:
request processing system
Priority:
Major
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
9 Apr 2014 at 18:19 UTC
Updated:
23 Feb 2015 at 00:34 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
sunComment #2
effulgentsia commentedComment #3
effulgentsia commentedLet's see what fails.
Comment #4
effulgentsia commentedAhem.
Comment #6
effulgentsia commentedWell, that was less than I expected :)
Comment #7
ParisLiakos commentednice!
I guess we should now mark it as deprecated and also completely remove
_current_path()?Comment #8
dawehnerYeah, paris is right.
Comment #9
webchickHm. Can we not just keep this function as a convenience wrapper? It's called a lot from preprocess functions and .module files according to #2239009-6: Remove public direct usage of the '_system_path' request attribute, and those things can never be dependency-injected.
Comment #10
tim.plunkettWe're keeping
current_path(), #7 was about the helper_current_path()(with the underscore)Comment #11
webchickOops. My mistake.
Comment #12
effulgentsia commentedLet's see what bot thinks of this.
Comment #13
ParisLiakos commentedany reason
arg()is moved around here? i dont really see the reason and it will unneedlessly break other patches, eg #788900: Deprecate and remove usages of arg()Comment #14
ParisLiakos commentedthis fixes #13
Comment #15
effulgentsia commentedThanks. I'm happy with #14. Since many hunks in the patch are mine, I'm refraining from RTBC'ing, especially since the Views pager related changes are not entirely trivial.
Comment #16
ParisLiakos commentedthe Views pager changes look valid to me and since moving around arg was not intentional, according to #15, i feel comfortable with RTBCing this
Comment #17
mondrakeThis is preventing pager navigation in case you have contextual filters enabled, and you enter a filter value in the 'Preview with contextual filters' textbox. I'm no expert here, but suspect that's somewhat to do with the path finally built and the routing system.
This somehow relates with #2066207: Views UI preview does not retain Contextual filters on pagination; in HEAD, pager navigation is working actually, but contextual filters are lost on any click on pager links. With #14 contextual filters are retained, but no pager navigation :(
The patch attached builds on #14 and attempts to solve the lack of navigation by moving the contextual filters to a query parameter instead of appending to the path. Not sure it's the right way to go though...
Comment #18
ParisLiakos commentedthis sounds like a different issue to me and seems an issue already exists. lets deal with this there?
Comment #19
mondrakeFine with me. Just let's be aware that view preview pager navigation with contextual filters won't work, temporarily.
Patch to review is still #14.
Comment #20
ParisLiakos commentedthis doesnt sound like it works?
Comment #21
effulgentsia commentedSince in HEAD, _current_path() and _system_path are not automatically sync'ed, the above hunk is not a no-op, and likely responsible for causing the functional change described in #17. Therefore, let's postpone this on getting the correct functionality into HEAD, along with a test, in #2066207: Views UI preview does not retain Contextual filters on pagination.
Comment #22
yesct commented#2251061-25: arg(0) returns path prefix with Language Path Negotiation enabled is wondering if we should wait on this, or try and fix _current_path()
Comment #23
yesct commentedComment #24
yesct commentednoting this is the @todo from #1183208: Remove variable_get('clean_url') and switch to index.php/path pattern for dirty URL support
core/includes/bootstrap.inc
Comment #25
dawehnerThat function does not longer exists, yeah!