Hello,

I made a "not legitimate but working" hack for the ones who have WURFL module for mobile checking. I know there are agent/browser checkers here and also another Mobile Key module, but they're no way near wurfl.

CommentFileSizeAuthor
#4 1214624_update_wurfl_themekey_6.x-3.x.patch1.79 KBchawl
#2 1214624_wurfl_themekey_6.x-3.x.patch1.73 KBmkalkbrenner
themekey_properties_wurfl_hack_20110711.patch899 byteschawl

Comments

mkalkbrenner’s picture

mkalkbrenner
Primary language German
Location 🇩🇪
commented
Project: ThemeKey Properties » ThemeKey
Component: Code » Code / API
Status: Needs review » Needs work

I will integrate this.

But I suggest to move the code into ThemeKey itself. There we have a mechanism to dynamically add properties if the module they depend on is installed. In this case there's no need to add something like function_exists().

Just create a file named themekey.wurfl.inc in the modules folder.

Additionally this approach could be ported to ThemeKey 7 later, because ThemeKey Properties has been merged.

mkalkbrenner’s picture

mkalkbrenner
Primary language German
Location 🇩🇪
commented
Status: Needs work » Needs review
StatusFileSize
new 1214624_wurfl_themekey_6.x-3.x.patch1.73 KB

@chawl:

It would be nice if you find some time to test the attached patch.

mkalkbrenner’s picture

mkalkbrenner
Primary language German
Location 🇩🇪
commented
Title: New property: wurfl: is_mobile » New property: wurfl: is_mobile_browser
chawl’s picture

chawl commented
StatusFileSize
new 1214624_update_wurfl_themekey_6.x-3.x.patch1.79 KB

Tx mkalkbrenner. In fact my original patch was for Themekey, but I don't want to mess up the core :)

Anyway patch is working ok, but omitting the function_exists wrap breaks the site for the ones who don't have WURFL module.

I'm sure many wurfl options will find their way in ThemeKey, so one day we should put some words about wurfl on ducumentation and alerts also, but for now I suggest your patch to be like this, because I've found no module check in your code :) and sorry if I miss smt.

mkalkbrenner’s picture

mkalkbrenner
Primary language German
Location 🇩🇪
commented

Anyway patch is working ok, but omitting the function_exists wrap breaks the site for the ones who don't have WURFL module.

No, have a look at themekey_scan_modules(). We only load the include files if the corresponding module is installed.

But maybe we should do this to get the code more robust:

diff --git a/modules/themekey.wurfl.inc b/modules/themekey.wurfl.inc
index 3a822a5..38213b0 100644
--- a/modules/themekey.wurfl.inc
+++ b/modules/themekey.wurfl.inc
@@ -50,8 +50,15 @@ function themekey_book_themekey_properties() {
  *   just a dummy
  *
  * @return
- *   string 'true' if client is a mobile browser, otherwise 'false'
+ *   string 'true' if client is a mobile browser, otherwise 'false',
+ *   or NULL if no value could be mapped
  */
 function themekey_wurfl_dummy2is_mobile_browser($dummy) {
-  return wurfl_get_requestingDevice()->getCapability('mobile_browser') ? 'true' : 'false';
+  $requesting_device = wurfl_get_requestingDevice();
+
+  if (is_object($requesting_device)) {
+    return $requesting_device->getCapability('mobile_browser') ? 'true' : 'false';
+  }
+
+  return NULL;

Could you please test the code and set the issue to 'reviewed ...' if it works like expected?

BTW it would be great to see more wurfl features to be integrated that way.

chawl’s picture

chawl commented

Ok, first a copy-paste issue. I think this
function themekey_book_themekey_properties()
should be this
function themekey_wurfl_themekey_properties()
in your code. As I reflexively corrected locally, I forgot to mention.

Yes, last mod is wiser and working perfectly, but there could be a bigger issue with the way the themekey_scan_modules() is called.

If you have enabled contrib module before TK or visit TK admin page after enabling, then everything goes into themekey_modules variable correctly. But if you disable the contrib module alone leaving the TK enabled, themekey_rebuild() is not invoked, variable table is not updated, unsupported inc is loaded erroneously and site breakes immediately with a fatal undefined function php error. As you are out of chance to visit TK admin page or update.php to invoke themekey_rebuild() again, a MySQL operation becomes inevitable.

Can this really be a caching issue or am I missing smt again?

Tx.

mkalkbrenner’s picture

mkalkbrenner
Primary language German
Location 🇩🇪
commented
Version: 6.x-3.x-dev » 7.x-1.x-dev
Assigned: Unassigned » mkalkbrenner
Status: Needs review » Patch (to be ported)

I committed the new feature to git. Now we need to port it to ThemeKey 7 when WURFL is ported: #1028232: WURFL Drupal 7 Version

@chawl:
You're right with you thoughts regarding the uninstallation of modules. But we should target this in a new issue because it effects all ThemeKey module integrations. For D7 this could easily be achieved by implementing hook_modules_disabled() but for D6 we need a different generic approach. See #1215656: Avoid errors if a third party module gets deactivated

anavarre’s picture

anavarre
Primary language French
Location 🇪🇺
commented

Subscribe

mkalkbrenner’s picture

mkalkbrenner
Primary language German
Location 🇩🇪
commented
Status: Patch (to be ported) » Closed (won't fix)

see #1321338: WURFL data is no longer open source

Have a look at the combination ThemeKey / Mobile Tools (https://drupal.org/project/mobile_tools) instead.