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

Wim Leers’s picture

Component: other » edit.module
Issue tags: +frontend performance, +Spark
nod_’s picture

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.

Wim Leers’s picture

That'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.

nod_’s picture

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.

Wim Leers’s picture

Status: Active » Closed (works as designed)

The 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(), override Drupal.edit.metadata and replace it with a localStorage-backed implementation.

Conclusion: a general optimization is impossible, a site-specific optimization is possible, and is allowed for by the current architecture.

Wim Leers’s picture

Wim Leers’s picture

Issue tags: +sprint

.

Wim Leers’s picture

Status: Active » Closed (works as designed)
Issue tags: -sprint