511699 - p12n: Make SHOWFOR configurable (for Firefox vs Fennec)

Status: VERIFIED FIXED

(support.mozilla.org :: General, defect)

Description

[Paul Craciunoiu [:paulc]]

For productization, we need showfor to be configurable so we can ship it for two different products, e.g. Firefox and Fennec

Comment 1

[Paul Craciunoiu [:paulc]]

James said he'll investigate the config file approach.

Comment 2

[James Socol [:jsocol, :james]]

Is there a list of what Fennec will need here? Obviously Mac vs Linux vs Windows is no longer applicable. What about versions?

Comment 3

[Paul Craciunoiu [:paulc]]

I'm guessing no. You think it would be possible to place the versions somewhere in the DOM when SHOWFOR is used?
That way, we could have a file that lists versions somehow. I'm not sure how Fennec versions and platforms can be detected yet. We'll need to talk to the team to figure that out.

Comment 4

[James Socol [:jsocol, :james]]

Attachment: php patch, v1

This patch moves the configuration (for the PHP part only) into a config file in scripts/showfor/config.php and comes with a config.php.dist (which is just the Firefox settings). This is a bit long, so let me explain...

Because of the way Tiki includes wikiplugin files, the config array is in the global scope. (I'm sorry. Another option was to wrap it in a global function that returns the array, but I thought this was easier to work with.)

The config array is extract()ed in the wikiplugin_showfor() function. This is less than ideal, but since the function constructs so many variable variables, it was the safest way to avoid identifier collisions. The other options were essentially equivalent to extract() anyway.

I added a new "os_aliases" array, and stripped out a few switch statements relating to the OS types, and replaced them with calls to wikiplugin_showfor_find_alias(). Another switch got replaced with the "os_spans" config array, which defines the sets of classes and their complements when SHOWFOR tries to hide content via the {DIV()} plugin.

I wasn't really sure how to handle browser/OS detection, so I left it as is. Modifications to the JavaScript can address those as needed.

I didn't touch the inner loops.

I modified the wikiplugin_showfor_find_alias() function so it doesn't require the real name to be both the array key, and listed as an alias, and added some documentation to it. (Probably too much, but I was being generous to future generations.)

I did NOT move the $types array into the configuration, because several of the inner loops seemed to depend on the specific values of "browser" and "os".

I added a blank line or two to make things easier to read, and a bunch of comments in the config.php.dist file explaining it. And finally, stripped off the closing ?> "just because".

Comment 5

[Paul Craciunoiu [:paulc]]

Comment on attachment 396801 [details] [diff] [review]
php patch, v1

Testing on a couple articles works fine for me. Functionality's there for OSes and versions, 2.0 is still hidden.

As a minor nitpick, the patch uses tabs for indentation, and since laura so mentioned we shouldn't, it'd be nice to change those.

Comment 6

[James Socol [:jsocol, :james]]

Attachment: patch, v1.0.1

Indenting in config.php.dist is now all with spaces. Tabs used in wikiplugin_showfor.php were already there.

Comment 7

[Paul Craciunoiu [:paulc]]

Comment on attachment 396844 [details] [diff] [review]
patch, v1.0.1

The same articles load fine :)

Comment 8

[James Socol [:jsocol, :james]]

Pav, can you give any insight into the onload behavior for Fennec? For example, on SUMO, we check to see if the user is running Firefox, and if so, try to start with the content for the current version and OS. If not, we fall back to the default browser.

Comment 9

[Paul Craciunoiu [:paulc]]

r50334 / r50335

Comment 10

[Stephen Donner [:stephend] Not actively reading bugmail]

We don't yet have Fennec in the list of supported user-agents for SUMO, right?  i.e., this isn't yet testable?

My user-agent is Mozilla/5.0 (Windows; U; WindowsCE 5.2; en-US; rv:1.9.3a1pre) Gecko/20090828 Fennec/1.0a3 (for when you want to test).

Comment 11

[Stephen Donner [:stephend] Not actively reading bugmail]

Yay, feeling good about this; tested:

* Firefox 2.0.0.20 - Windows XP
* Firefox 3.0.13 - Windows Vista, Ubuntu
* Firefox 3.5.2 - Vista, Ubuntu, Mac OS X
* IE 6 - XP
* IE 7 - Vista
* IE 8 - Vista
* Opera 9.62 - Windows 7
* Safari 4.0.3 - Mac OS X
* Google Chrome 2.0.172.43 - Windows Vista

Verified FIXED -- feel free to double-check my work, someone!

Comment 12

[James Socol [:jsocol, :james]]

scripts/showfor/config.php.dist is already set up for SUMO, it just needs to be copied to scripts/showfor/config.php