Follow-up to #2480811: Cache incoming path processing and route matching
Inbound and Outbound path processors are supposed to take a path that always has a leading /. The InboundPathProcessorInterface documents this fact. OutboundPathprocessorInterface does not. Let's fix that.
Update the docblock on OutboundPathProcessorInterface that $path must have a leading /. See InboundPathProcessorInterface for an example.
| Issue category | Bug because the API is unclear. |
|---|---|
| Issue priority | Normal because nothing is seriously broken, just slightly broken. |
| Disruption | This is no longer an API change in practice, and even if something is affected it's already wrong if it's not using a leading-/ path so the bug to fix would be there anyway. Just a docfix. Move along. :-) |
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | clarify_pathprocessor-2508037-8.patch | 1.64 KB | hussainweb |
Comments
Comment #1
dawehnerAdding a really similar issue: #2430805: Fix CurrentPathStack::getPath() documentation that says it has no leading / (and make it easier to actually get that?)
Comment #2
catchYes this turned me around several times on the route caching patch.
Comment #3
Crell commentedI'm going to take #2 as endorsement to move forward here. :-) To avoid unnecessary rerolls, marking it postponed on the original issue; once that's in this should be a pretty simple patch.
Also fixing status to align with the beta summary; I probably fat-fingered when filing.
Comment #4
catchI think you're underestimating how broken this is. "We've been trying to standardize on leading-/ for paths" was never codified anywhere or applied consistently.
The thing that turned me around on the other patch was that path alias storage stores without a leading slash.
Additionally AliasManager expects a path with no leading slash.
So to get rid of unnecessary trimming and adding of '/', this will need to be standardized. Not just the path processors themselves.
Then I did a quick bit of testing of the path alias storage vs. menu UIi and found #2509300: Path alias UI allows node/1 and /node/1 as system path then fatals. Opened that as a separate critical bug since it's a data-integrity issue, but it might need to be fixed at the same time as this.
Comment #5
Crell commentedOriginal blocker is in, but leaving as postponed on #2509300: Path alias UI allows node/1 and /node/1 as system path then fatals for now.
Comment #6
dawehnerBack to active.
Comment #7
Crell commentedThe previous issues have fixed up all the bugs here, it looks like, save for documentation. InboundPathProcessorInterface now specifies leading-/, but outbound does not. Therefore, turning this into a novice docfix issue.
Comment #8
hussainwebGetting this started. There are some other small changes in the docblock as well.
Comment #10
Crell commentedHey, you're not a novice! :-)
Comment #11
hussainwebI sure am. Every time I get back to issue queues, everything is very different. :)
Comment #14
hussainwebBack to RTBC as per #10.
Comment #17
hussainwebResetting to RTBC as per #10 after random failure.
Comment #20
hussainwebOnce again, Resetting to RTBC as per #10 after random failure.
Comment #21
Crell commentedPay attention, Drupal.org!
Comment #22
webchickCommitted and pushed to 8.0.x. Thanks!