540107 - Fix SHOWFOR detection for N810 and N900

Status: VERIFIED FIXED

(support.mozilla.org :: Mobile, task, P1)

Description

[James Socol [:jsocol, :james]]

The current SHOWFOR configs on mobile.sumo are based on old values for the User-Agent strings on the Nokia devices.

https://bugzilla.mozilla.org/show_bug.cgi?id=517086#c12

Has the current ones. We need to update the SHOWFOR configs to match.

Comment 1

[James Socol [:jsocol, :james]]

Attachment: update wikiplugin_showfor.js.dist-fennec with correct UA for N810

Replaces the "N810" string with "N8xx" to match the value given in bug 517086 comment 12.

Comment 2

[James Socol [:jsocol, :james]]

r61842.

To see this on stage, we'll need to get someone to copy webroot/js/wikiplugin_showfor.js.dist-fennec to wikiplugin_showfor.js (unless that happens magically?).

Comment 3

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

(In reply to comment #2)
> r61842.
> 
> To see this on stage, we'll need to get someone to copy
> webroot/js/wikiplugin_showfor.js.dist-fennec to wikiplugin_showfor.js (unless
> that happens magically?).

Does this need an IT bug?

Comment 4

[James Socol [:jsocol, :james]]

(In reply to comment #3)
> Does this need an IT bug?

Apparently. (The mobile.support-stage SHOWFOR JavaScript wasn't even the dist-fennec version.) Filed bug 545180.

Comment 5

[Chris Ilias [:cilias]]

I have created https://mobile-support-stage.mozilla.com/en-US/kb/SHOWFORtest
The markup is:
{SHOWFOR(os=N810)}N810{SHOWFOR}
{SHOWFOR(os=N900)}N900{SHOWFOR}
{SHOWFOR(os=Windows Mobile)}Windows Mobile{SHOWFOR}

Comment 6

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

(In reply to comment #5)
> I have created https://mobile-support-stage.mozilla.com/en-US/kb/SHOWFORtest
> The markup is:
> {SHOWFOR(os=N810)}N810{SHOWFOR}
> {SHOWFOR(os=N900)}N900{SHOWFOR}
> {SHOWFOR(os=Windows Mobile)}Windows Mobile{SHOWFOR}

Chris, James said you need to add:

{SHOWFOR(spans=on)/}, I think, to the top of that article.

Comment 7

[Chris Ilias [:cilias]]

{SHOWFOR(spans=on)/} just adds support for spans, using the DIV tag rather than SHOWFOR. e.g. {DIV(class=mac,type=span)}Cmd{DIV}

It is not needed for SHOWFOR tags to work.

Comment 8

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

OK, well, https://mobile-support-stage.mozilla.com/en-US/kb/SHOWFORtest still doesn't work, and I'd like to test this.  How can we move forward?

Comment 9

[Chris Ilias [:cilias]]

Sorry if there's a misunderstanding.
{SHOWFOR(spans=on)/} was added to https://mobile-support-stage.mozilla.com/en-US/kb/SHOWFORtest as well as a line:
{DIV(class=n810,type=span)}n810{DIV}

It shouldn't be needed. If it doesn't work, I would say this bug is not fixed.

Comment 10

[James Socol [:jsocol, :james]]

Looks to me like the wrong config.php.dist was used. Filed bug 548140.

Comment 11

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

It's not correctly auto-detecting my N900; reopening.

Comment 12

[James Socol [:jsocol, :james]]

Attachment: use correct UA detection strings, and switches default from n810 to n900

Looking at more recent UA strings, it looks like the information I used in comment 1 was wrong. This patch uses strings from more recent builds.

I also swapped the default in scripts/showfor/config.php.dist-fennec from the N810 to the N900. This part of the patch can be applied or not. When I wrote the original config file, the N900 had not been released yet. That's obviously no longer true.

Comment 13

[James Socol [:jsocol, :james]]

Attachment: substrings in the right spot

I switched the wrong values in the last patch. Fail. Clearly it's time to head home and get some sleep.

Comment 14

[James Socol [:jsocol, :james]]

r63161. Filed bug 548340 to get the .dist-fennec's copied again (and hopefully set up to automatically copy in the future).

Comment 15

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

For whatever reason, the following N810 user-agent isn't being auto-detected:

Mozilla/5.0 (X11; U; Linux armv61; en-US; rv: 1.9.2.1) Gecko/20100126 Firefox/3.6.1 Fennec/1.0.0

Comment 16

[James Socol [:jsocol, :james]]

This _does_ work on Windows Mobile, leaving me stumped at the moment.

Comment 17

[James Socol [:jsocol, :james]]

Attachment: *expletive deleted*

So it turns out the UA for the N810 is armv6L, not armv6(one). But that's changed since they first set the UAs, so this just looks for "armv6" which should be specific enough to N810s.

Comment 18

[Laura Thomson :laura]

Comment on attachment 428817 [details] [diff] [review]
*expletive deleted*

rofl.  but it works.

Comment 19

[James Socol [:jsocol, :james]]

r63273. Reopening bug 548340.

Comment 20

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

James, you're going to hate me (temporarily, I hope), but this still isn't working.  On an N810, it's still defaulting (with cookies deleted) to the N900.

Comment 21

[James Socol [:jsocol, :james]]

Attachment: i'm an idiot

This patch just removes the second copy of the file inside the file. I guess when I originally did this I had a vast misunderstanding of how patch works. Also, I hate patch.

Comment 22

[Laura Thomson :laura]

Comment on attachment 428992 [details] [diff] [review]
i'm an idiot

Not an idiot, or at least if you are I'm one too (I've done this exact thing).

Comment 23

[James Socol [:jsocol, :james]]

r63484. Reopening the IT bug yet again.

Comment 24

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

Verified FIXED; tested on both the N900 and the N810 (and saw it working on WinMo).