it'd be nice if the weburl field supported internal links, instead of requiring an external link (domain or IP). i can guess that the intention is for people to use a noderef if they want to use an internal link, but those don't work with urlalias'ed paths, menus, tabs, etc. i think a URL field should support internal links, too.

the primary change would have to be in weburl_validate_weburl(). it's currently doing a big preg_match() and populating a $url object with different fields (protocol, hostname, port, IP). this object is the return value if the URL parsed correctly. otherwise, the funciton returns false. however, nothing ever uses these parsed fields. the only call sites are just testing to make sure weburl_validate_weburl() returns non-false.

i'm not sure what the motivation is for all this parsing complexity, especially since a) no one uses it and b) it prevents internal links from working.

attached is a naive patch that keeps the giant regexp to validate external links, but that has (brain-dead) logic to "validate" internal links if the big regexp fails (searches for a '/' in the string, which, upon futher thought, wouldn't work in some urlalias cases). i'm not sure how we can/should validate internal links, since "calendar" could be a valid relative link. hrm. anyone have any bright ideas?

anyway, here's the patch (which also converts a few tabs to spaces in the rest of the function, i can roll that separately if folks prefer) as a place to start from. seems like weburl_validate_weburl() needs more thought, so i'm not even going to bother setting the status to "patch needs review" or "code needs work". ;)

thanks,
-derek

CommentFileSizeAuthor
cck_weburl_validate_weburl.patch584 bytesdww
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

robertDouglass’s picture

Internal links aren't fully qualified URLs. My use for this field type is to make sure that the value is a fully qualified URL. Morbus wants to extend it to handle the full range of the URI standard, which is fine with me. Simply checking whether it has a forward slash and returning true if it does will render it useless for its original purpose.

Are you looking for a field that takes a Drupal path like node/23 and makes a link out of it?

dww’s picture

Simply checking whether it has a forward slash and returning true if it does will render it useless for its original purpose.

right. that's why i spoke so poorly of my own patch in my initial post. ;) i'm just trying to understand the original purpose of this field, since nothing looks at the results of this parsing.

Are you looking for a field that takes a Drupal path like node/23 and makes a link out of it?

well, if all i wanted was node/23 and a link, i could just use a nodereference. i was thinking more of a link to something like admin/settings/foo. i suppose we could just advocate that people use a simple text field with html filtering on, and put in the <a href="admin/settings/foo"> themselves. that just seems a little unfriendly for end users. plus, i can easily imagine people getting confused by this field type (esp given the flexinode field_url folks will associate it with) -- if this is a field for holding urls, why don't relative URLs work, too?

i don't have a fully-formulated proposal on this yet. i was just curious what's the point of the existing parsing code in weburl if no one ever looks at the results. i suppose the answer is "it's still a work in progress", which makes sense. but, i'm still worried about an unneccessary differentiation between external and internal links.

-derek