Follow-up to #2343661: Rename l() to _l() and url() to _url(), and document replacements
Problem/Motivation
_l()
and _url()
circumvent the routing system, and their replacements are not backward-compatible, so they should be removed before release.
Proposed resolution
Remove _l()
and _url()
before release.
Remaining tasks
- function _url() and function _l() themselves in common.inc: covered by this issue.
- Various _url() and _l() calls in Views, covered in #2409209: Replace all _url() calls beside the one in _l(). Blocker for this one.
- Tests for _l() in ./core/modules/system/src/Tests/Common/UrlTest.php: were removed in #2364161: Replace nearly all existing _l calls.
- A call to _l() in ./core/modules/views/src/Plugin/views/field/FieldPluginBase.php, covered by #2404603: Add proper support for Url objects in FieldPluginBase::renderAsLink(), so we can remove EntityInterface::getSystemPath(). Blocker for this one.
- A call to _l() in ./core/modules/views/src/Plugin/views/field/Url.php, covered in #2368653: Replace _l in all places (3) besides one.. Blocker for this one.
- Various references to both in documentation: #2410497: Update remaining url() and _l() calls in comments/documentation but not a blocker to this one, and only a normal or possibly major priority; mistaken inline docs would not hold up the release.
Task | Novice task? | Contributor instructions | Complete? |
---|---|---|---|
Find the change records that will need to be updated. | List links to them in the summary. | ||
Update change records for the API changes | Instructions for drafting a change record might be helpful, but note this is *updating* existing records. |
User interface changes
None.
API changes
_l()
and _url()
are removed
Postponed until
#2343661: Rename l() to _l() and url() to _url(), and document replacements
#2345725: Query parameters are not decoded the same as the path portion of a URL
#2344487: Provide special route names for <current> and <none>
#2364157: Replace most existing _url calls with Url objects
#2368323: Replace _l() in PathController::adminOverview()
#2368653: Replace _l in all places (3) besides one.
#2280961: (Views)FieldPluginBase::advancedRender() calls SafeMarkup::set() on a string that it doesn't know to be safe
#2404041: Replace _l() calls in file module
#2409209: Replace all _url() calls beside the one in _l()
#2410499: Remove remaining _l() calls in Views
Comment | File | Size | Author |
---|---|---|---|
#28 | remove_l_and_url-2343669-28.patch | 9.33 KB | mpdonadio |
Comments
Comment #1
xjmComment #2
xjmComment #3
xjmComment #4
xjmWe'll need to finalize https://www.drupal.org/node/2346779 once this lands.
Comment #5
dawehnerThis depends on two issues at least atm.
Comment #6
geerlingguy CreditAttribution: geerlingguy commentedAlso, FYI to anyone who might be burned by not being able to make arbitrary links (e.g. in .install files) to drupal paths, You can do so by using
base://
. This will work for any internal path that may not yet be in the routing system. You shouldn't normally do this... but it might be necessary in some circumstances:Comment #7
catchComment #8
dawehnerWell, there is at least #2368323: Replace _l() in PathController::adminOverview() as well.
Comment #9
xjmComment #10
YesCT CreditAttribution: YesCT commentedupdated summary with the issues blocking this, and a remaining task. but might need more details added to the summary for when this is unpostponed.
Comment #11
Wim Leers#2368323: Replace _l() in PathController::adminOverview() landed. :)
Comment #12
Wim LeersBut isn't this also blocked on #2368653: Replace _l in all places (3) besides one. and #2280961: (Views)FieldPluginBase::advancedRender() calls SafeMarkup::set() on a string that it doesn't know to be safe ?
Comment #13
pcambraAdded these two as well:
#2404043: Replace _l() calls in FieldPluginBase
#2404041: Replace _l() calls in file module
Comment #14
rpayanmHello someone can tell me what mean [PP-1] on the issue title? Thank.
Comment #15
cilefen CreditAttribution: cilefen commentedPostponed, one level deep.
Comment #16
YesCT CreditAttribution: YesCT commentedI think it is postponed on 4 issues now, adding them to the list.
Comment #17
webchickAdding a few more dependent sub-issues I found via #2364157: Replace most existing _url calls with Url objects.
Comment #18
mpdonadioRemoved issue that no longer blocks this.
Comment #19
webchick#2364157: Replace most existing _url calls with Url objects went in, so one down. :)
Comment #20
webchickActually, for giggles and grins I did a "whole word" grep for _url( and _l(. Here's where we're currently at:
_url(): 17 usages.
_l(): 41 usages.
AFAICS all of these are covered by the following:
So by my count, we're now down to PP-3. Updated the issue summary with remaining tasks.
Comment #21
webchick#2368653: Replace _l in all places (3) besides one. down, 2 to go!
Comment #22
webchickOk, #2409209: Replace all _url() calls beside the one in _l() is in now, so we're just down to one blocker at #2404603: Add proper support for Url objects in FieldPluginBase::renderAsLink(), so we can remove EntityInterface::getSystemPath() which is currently NW but has been RTBC at least once.
Comment #23
mpdonadio#2404603: Add proper support for Url objects in FieldPluginBase::renderAsLink(), so we can remove EntityInterface::getSystemPath() will be getting a patch tonight that should address all concerns. I just need to post it and explain a bit.
Comment #24
mpdonadioSo, I got antsy, and took the latest patch from #2404603: Add proper support for Url objects in FieldPluginBase::renderAsLink(), so we can remove EntityInterface::getSystemPath() and made a test branch from it. PhpStorm reported no uses of _url() or _l(), other than themselves. As a test, I removed both from common.inc, made a patch, and it comes up green.
Probably need to do a few greps/searches to catch uses in comments (but IS says that shouldn't hold up this issue), so this is looking good.
Updated IS since one of the Remaining Tasks was covered by an issue that has been done for a while now.
Comment #25
mpdonadioI think all blockers are gone now.
Comment #26
dawehnerToo bad, I'd like to RTBC this already.
Comment #27
dawehner@mpdonadio
Can you roll a small patch, please?
Comment #28
mpdonadioReroll b/c git a git problem.
Comment #29
dawehnerPerfect, the documentation is reflected in another issue.
Comment #32
star-szrBack to #29's RTBC because it came back green.
Comment #33
alexpottBye, bye, old friends.
Committed a7e819e and pushed to 8.0.x. Thanks!
Comment #35
hussainwebOpened #2424525: Remove references to _l() and _url() for a followup to remove references to _l() and _url() from comments and doxygen.