Using library_load, integration files are currently loaded before the library itself is loaded. In all cases (CSS, JS and certainly PHP), I think you'd want your integration files to come later, since they will patch, extend, or otherwise leverage the library files themselves. This patch moves the integration file loading to after loading the actual library files.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rbayliss’s picture

Status: Active » Needs review
Pol’s picture

Status: Needs review » Reviewed & tested by the community

Patch working, thanks !

tstoeckler’s picture

Category: task » bug
Priority: Normal » Major

For PHP files, this is a pretty big bug report actually, as you can't use PHP functions or classes from the library you depend on... #fail

Will commit this soon. This is technically a BC-break, but, again, I totally see this as a bug-fix.

tstoeckler’s picture

Status: Reviewed & tested by the community » Fixed

Sorry for letting this rot for so long!

Even though this is a change in behavior I could not think of any (realistic) way this would actually *break* stuff, so I committed this to all three current branches:
8.x-3.x: http://drupalcode.org/project/libraries.git/commit/570ce1284b3cc6a706db1...
7.x-3.x: http://drupalcode.org/project/libraries.git/commit/59ef3b21b615442827600...
7.x-2.x: http://drupalcode.org/project/libraries.git/commit/67a7bb3d74ea4c9650f68...

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

tstoeckler’s picture

Issue summary: View changes
Status: Closed (fixed) » Active

I in fact changed my mind on this. We shouldn't be doing any API changes in 7.x-2.x. Although loading integration files after the library files might still be the primary use-case, I can think of a couple use-cases for the opposite as well. So let's fix this properly in 3.x (i.e. allow both) and provide a backwards compatible way for this in 2.x. Patch coming up.

tstoeckler’s picture

Status: Active » Needs review
FileSize
5.18 KB

Here we go.

tstoeckler’s picture

Version: 7.x-2.x-dev » 7.x-3.x-dev
Status: Needs review » Active
FileSize
5.18 KB

Committed the attached patch to 7.x-2.x.

http://drupalcode.org/project/libraries.git/commit/8d57cc2eaca69f509f73d...

In the previous patch the test coverage was passing but not really valid. Because the parent tests get executed (potentially, depending on Batch API) in one request, the library file might already have been loaded, which makes the test pass even if the files are loaded incorrectly.

Moving to 7.x-3.x.

tstoeckler’s picture

Oops here's the actual patch.

Elin Yordanov’s picture

Version: 7.x-3.x-dev » 7.x-2.2
Status: Active » Needs review
FileSize
1.08 KB

Since the latest update, because of the above patch, I am getting PHP notices like

Notice: Undefined index: post-load integration files

Attached is the revisited patch, please review. (I wasn't sure if I should open a new issue for that, since the issue is related to this issue, actually is the consequence of the committed patch with the latest update.)

Status: Needs review » Needs work

The last submitted patch, 10: 1855918-10-integration-files-order.patch, failed testing.

tstoeckler’s picture

Hm... thanks for the patch.

I don't see why those notices would appear, though. Have you tried a drush cc all? If that doesn't help could you help provide some debugging info, please, that would be awesome. It should be sufficient to just stick a debug(debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS)) at the top of libraries_load_files(). Thanks!

davidneedham’s picture

It may be unrelated, but could you be using Colorbox, pc-wurm?

I was seeing the notices you're talking about and with some digging it eventually it led me here: https://drupal.org/node/2150665. In my case, updating Libraries to 7.x-2.2 and downgrading colorbox to 7.x-2.4 made all notices go away and restored all functionality.

amitgoyal’s picture

Status: Needs work » Needs review
FileSize
1.05 KB

@davidneedham - I am having Libraries 7.x-2.2 and Colorbox to 7.x-2.5 and there are no such notices. So these notices might be coming due to some other reason.

@tstoeckler - Although I couldn't reproduce the notices issue but as the notices are coming in libraries.module fie so it may make sense to add extra isset() check there. Attaching here the updated patch as the 1855918-10-integration-files-order.patch was not getting applied.

tstoeckler’s picture

Version: 7.x-2.2 » 7.x-3.x-dev
Status: Needs review » Active
tstoeckler’s picture

Version: 7.x-3.x-dev » 7.x-2.x-dev
Status: Active » Fixed

7.x-3.x is very much outdated right now, so going back to fixed for now.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.