Preprocess is misdefined in system_library()

@sun - This is a bug that exists in core. Updating drupal_add_library() to add a feature that introduces an API change which there is not even a patch for yet is something else.

Let's get this bug fix in here and see where the feature/API change happens on the other issue. It may not happen until D8. The bug fix should not be held up because of that.

@sun I do understand what you are pointing out with drupal_add_library(). I see a bug in the current implementation and an issue with the nature of drupal_add_library(). I am clear on what you mean.

But, we are after the code freeze, what you propose is an API change, and we do not know if that API change will be allowed in D7. So, in the meantime lets fix this bug.

If the other issue gets handled, great. If not, this needs to be fixed.

The other issue is not moving on a clear path, is lacking code, and is running out of time. It may well have to wait until D8. This bug needs to be fixed in D7.

@sun nice catch on the bug in drupal_render_cache_get(). This can be fixed without changing the API. Is there a reason the API change cannot wait until D8?

@sun I have two issues with changing the API:

1. The API is frozen. Not everything is perfect. If we wait for everything to be perfect we will never release.
2. Adding an $options array with weight and preprocess at the caller level is not so simple. There are architectural decisions that have been made and would need to be altered and discussed. For example, should the caller or definer of a library specify the weight? Should the definer provide a default weight that can be overridden? Or, something else? This takes time to work out and is an API change to implement. By our place in the cycle this waits for D8.

I'm inclined to say the functionality is what it is. We are now fixing bugs in that functionality. New features or feature changes go in D8. That means the $options array needs to wait and bug fixes go in for now. Unless Dries or webchick say differently and give this change a pass.

@tstoeckler The render pipeline issue is a bug in the current system. The $options is something different and it is in fact a place the definer may know more.

For example, say a library consists of 3 JavaScript files with a specific dependency ordering. A library is not a file, it's a collection of JS and CSS. The definer can know the dependency order of the JS files and provide the proper weighting. How can the caller of the library properly specify the weight for all the files? Should the caller even care about the ordering of the dependent files or just that adding the library works as expected?

This is not a simple problem. Working through it means we need to make certain assumptions. It is a change in functionality rather than a bug that needs to be fixed. The current system works on a different set of assumptions from the one he specified.

The preprocessing issue is interesting because the preprocess changes were brought in late in the game. They are, also, not so simple to deal with. Should all the JS files in a library be preprocessed? If two invocations call two different preprocess settings who wins?

While I do believe we need to work through all of this that does not mean it is right for D7. We are frozen and bug fixing. Working out the ins and outs of features is done unless granted an exception.

@Damien The current preprocess effort is about limiting the number of preprocess files to avoid downloading duplicates. So, all the common stuff should be included on every page and part of a preprocessed file while the files included on a limited number of pages are not preprocessed. This avoids downloading duplicate content. If you disagree with this please see #769226: Optimize JS/CSS aggregation for front-end performance and DX.

Until Dries or Webchick give the $options direction a go ahead I'm pursuing fixing the bugs in the current implementation. For another else please open a different issue as this is not the appropriate place.

It turns out there is no issue in #5 and the usage is consistent. Both drupal_render and drupal_render_cache_get use the same arguments for drupal_process_attached. If there is a library in either case they are both passed into drupal_add_library which is consistent in its use both times. There should not be an issue of weights. That means the original patch of system_library_preprocess_oops.patch is still good. I tested and it still applies against head.

Can we please fix this issue and deal with other aspects, features, and possibilities in other issues.