I am getting this notice, and update will not run:

Notice: Constant VERSION already defined in include_once() (line 18 of xxxx/sites/all/modules/jwplayermodule/jwplayermodule.module).

When I go to line 18 and comment out this line:

define("VERSION", "version");

It runs, though I still am having a (unrelated?) issue with not being able to play on ipod/iphone. #1219554: "Video could not be loaded" on iOS devices

No idea what to make of this.

Comments

Sk8erPeter’s picture

Priority: Normal » Major

I have to confirm this bug, it's really annoying and this is just a nonsense, as it doesn't make sense that a Drupal core constant is predefined and "overridden", and which should rather be defined in system.module file. I spent a few hours finding the source of my problem, and posted a question and "solution" with screenshots on drupal.stackexchange.com: "Drupal 6 version - in “Status report” it just gives back the string “version”, which causes problems when querying Drupal version (e.g. for modules)". This line should be simply taken out from this module.

JW Player’s picture

Interesting that I didn't get any warnings in my installation. In fact the version number shows up correctly for me in the Drupal status report. Had I seen an issue locally this never would have been released like this.

In any case it was my oversight for not namespacing this constant. However, looking at system.module it should have been namespaced there as well like the other constants to avoid this collision. As you say, in most cases a module shouldn't easily cause a conflict like this.

This will be fixed in the next version without question. I appreciate your diligence.

Sk8erPeter’s picture

Thanks for your feedback!

Interesting that I didn't get any warnings in my installation.

I didn't get any warnings after installing the module either. I just got the error messages when looking in the "Status report" menu. Webform checked the Drupal core version number, but it was already overridden because of your code.
As you can see in original post and my linked article, Drupal version was equal to "version" string as mentioned earlier.

In any case it was my oversight for not namespacing this constant. However, looking at system.module it should have been namespaced there as well like the other constants to avoid this collision. As you say, in most cases a module shouldn't easily cause a conflict like this.

Yepp, but to be honest, I still don't really understand why this line is needed at all. What can you use the VERSION constant for even in your own namespace if it's defined like this: define("VERSION", "version"); ? And why messing with namespaces when it can also cause PHP compatibility problems when someone is still using PHP version less than 5.3 on the server (e.g. 5.2.16 - like me)?

This will be fixed in the next version without question.

Can you already tell when this will be issued? :)

JW Player’s picture

@Sk8erPeter,

I haven't been able to get to this yet as I've been off sick for quite a while. I will try and get to this as soon as I can.

Namespace was perhaps not the correct word. I just meant prepending something to the constant name to make it more unique and less likely to collide with other constants.

Again, thanks for your efforts.