Problem/Motivation
Over at #1824500-94: In-place editing for Fields, @catch voiced very solid concerns about Edit module breaking render caching of entities because of edit_preprocess_field()
(which adds metadata to every rendered field), plus the fact that would slow down page rendering too much. The solution we settled on (#1824500-95: In-place editing for Fields) was to retrieve the metadata in an AJAX callback; the result can be seen in #1824500-95: In-place editing for Fields. (Note that the result is not an AjaxResponse
, but a JsonResponse
, i.e. no Drupal AJAX command framework commands, but simply JSON.).
Now this is much better already, but what's still problematic is that this AJAX request is now being requested on every page where there are fields annotated with metadata. So for many pages, there will actually be 2 requests to the server instead of one. Of course, the first one can now conceivably be cached by a reverse proxy such as Varnish, but it's still two requests.
Proposed resolution
TBD
Remaining tasks
Figure out a solution that triggers the metadata AJAX call less often; in other words: figure out heuristics that allow us to *not* perform the metadata retrieval.
Examples:
- more granular permissions: permission to edit just node fields, or just user fields, etc., which would allow the JS to determine no metadata is needed when only things that are not editable by the current user exist on the page
- caching in localStorage, trivial to implement, but then the problem becomes cache invalidation — keep in mind that Drupal allows for arbitrarily complex (or weird) access permissions, which might mean that caching metadata in localStorage becomes pointless
User interface changes
None.
API changes
None.
Comments
Comment #1
Wim LeersComment #2
nod_how about we get the metadata when we toggle the edit mode?
It might make sense for some users to have the metadata fetched all the time on load so an option like we have for the overlay might work out.
D7 version fetches metadata when a custom event is fired, should be easy enough to port to D8.
Comment #3
Wim LeersThat's no longer how D8 works as of #1874664: Introduce toolbar level "Edit" mode that shows all contextual links, so that's no longer possible :(
There's no longer a need to toggle "Edit mode" before you can start in-place editing something. I.e. you can hover over a node, click its contextual links, and there will already be a "quick edit" contextual link…
Of course, we could say that we want to get rid of this on-hover behavior, in which case what you describe becomes viable again, but Bojhan (and others too IIRC) thought it was important to keep the number of steps as low as possible on non-touch devices.
Comment #4
nod_Because Drupal "allows for arbitrarily complex (or weird) access permissions" there is nothing much we can do. Cache invalidation would be a pain and contrib module will have a hard time if they need to integrate their permission things with the edit module.
Something always good would be to reduce the size of the AJAX request but that's already an open issue elsewhere.
Comment #5
Wim LeersThe craziest things are indeed possible.
However, the majority of sites won't have crazy things there. So it should be possible for a contrib module to override this. But that's already possible: use
hook_library_alter()
, overrideDrupal.edit.metadata
and replace it with alocalStorage
-backed implementation.Conclusion: a general optimization is impossible, a site-specific optimization is possible, and is allowed for by the current architecture.
Comment #6
Wim LeersI'm working on a D8 contrib module that does precisely what is described in #5.
To do that, I've done #2005644: Use client-side cache tags & caching to eliminate 1 HTTP requests/page for in-place editing metadata, introduce drupalSettings.user.permissionsHash and will do #1980744: Turn edit/metadata into a JsonResponse again, to allow contrib to implement client-side caching of metadata.
Comment #7
Wim Leers.
Comment #8
Wim LeersAnd here's the module: http://drupal.org/project/edit_metadata_cache. Note that for it to work, we need #2005644: Use client-side cache tags & caching to eliminate 1 HTTP requests/page for in-place editing metadata, introduce drupalSettings.user.permissionsHash and #1980744: Turn edit/metadata into a JsonResponse again, to allow contrib to implement client-side caching of metadata to land. My claim in #5, "is allowed for by the current architecture" was right conceptually, but not in the details.