Notice thrown if option 'external' not set

kndr - June 4, 2009 - 11:57
Project:Global Redirect
Version:6.x-1.x-dev
Component:Code
Category:bug report
Priority:normal
Assigned:Unassigned
Status:needs work
Description

In function globalredirect_init() option 'external' should be set to FALSE (I think) since following message is shown: "There is a notice: Undefined index: external in language.inc on line 109." (locale and i18 module are enabled). Look at core url() function, where language_url_rewrite() is called - 'external' option is always set before call to language_url_rewrite();

AttachmentSize
external.patch618 bytes

#1

smk-ka - July 14, 2009 - 16:32
Status:needs review» reviewed & tested by the community

Yes, this is annoying. Re-rolled patch with nothing else than whitespace fixed, so it's RTBC.

AttachmentSize
481856-notice-D6.patch 860 bytes

#2

ranza - July 23, 2009 - 11:16

i actually just fixed line 109 by doing if(!isset($options['external'])) {} insted of !$options['e... in language.inc

#3

mrfelton - October 3, 2009 - 10:37

That sounds like perhaps a patch to core is what is really needed.

#4

smk-ka - October 4, 2009 - 11:44
Title:Option 'external' should be set before language_url_rewrite() is called» Notice thrown if option 'external' not set
Project:Global Redirect» Drupal
Version:6.x-1.x-dev» 7.x-dev
Component:Code» language system
Status:reviewed & tested by the community» needs review

Agreed, let's try that: language_url_rewrite() should not throw a notice if $options['external'] is not set.

AttachmentSize
481856-notice-external.patch 630 bytes
481856-notice-external-D6.patch 642 bytes

#5

Damien Tournoud - October 4, 2009 - 11:49

Erm. Why not fixing that in Global redirect? You are calling a core function, it's your responsibility to call it correctly.

#6

sun - October 4, 2009 - 11:54
Status:needs review» reviewed & tested by the community

Because it's bad code? $options['external'] is an optional property.

#7

Damien Tournoud - October 4, 2009 - 11:57

$options['external'] is an optional property.

It is not, it is set by default in url(), before calling language_url_rewrite().

#8

Damien Tournoud - October 4, 2009 - 12:00
Project:Drupal» Global Redirect
Version:7.x-dev» 6.x-1.x-dev
Component:language system» Code
Status:reviewed & tested by the community» needs work

It makes zero sense not to fix that in Global redirect first.

#9

mrfelton - October 4, 2009 - 18:30

Nothing in http://api.drupal.org/api/function/language_url_rewrite/6 mentions that 'external' is a required option. Surely, just because the parameter is set in url() before the function is called, that doesn't make it a mandatory parameter - that's just the way that url() uses it.

EDIT: Also note that in http://api.drupal.org/api/function/url/6, where the various options are defined (on the language_url_rewrite page it says that the options are the same as for url()) the entire $options array is optional, and if (!isset($options['external'])) is used the check for the presence of the 'external' option.

 
 

Drupal is a registered trademark of Dries Buytaert.