Fix SHOWFOR detection for N810 and N900

VERIFIED FIXED in 1.5.2

Status

support.mozilla.org
Mobile
P1
normal
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: jsocol, Assigned: jsocol)

Tracking

unspecified
1.5.2
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(4 attachments, 1 obsolete attachment)

(Assignee)

Description

8 years ago
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.
(Assignee)

Updated

8 years ago
Assignee: nobody → james
See Also: → bug 517086
Target Milestone: --- → 1.5.2
(Assignee)

Comment 1

8 years ago
Created attachment 425497 [details] [diff] [review]
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.
Attachment #425497 - Flags: review?(laura)

Updated

8 years ago
Attachment #425497 - Flags: review?(laura) → review+
(Assignee)

Comment 2

8 years ago
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?).
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(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?
(Assignee)

Updated

8 years ago
Depends on: 545180
(Assignee)

Comment 4

8 years ago
(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.
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}
(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.
{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.
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?
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.
(Assignee)

Comment 10

8 years ago
Looks to me like the wrong config.php.dist was used. Filed bug 548140.
Depends on: 548140
(Assignee)

Updated

8 years ago
Depends on: 548152
It's not correctly auto-detecting my N900; reopening.
Status: RESOLVED → VERIFIED
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 12

8 years ago
Created attachment 428590 [details] [diff] [review]
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.
Attachment #428590 - Flags: review?(laura)
(Assignee)

Comment 13

8 years ago
Created attachment 428593 [details] [diff] [review]
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.
Attachment #428590 - Attachment is obsolete: true
Attachment #428593 - Flags: review?(laura)
Attachment #428590 - Flags: review?(laura)

Updated

8 years ago
Attachment #428593 - Flags: review?(laura) → review+
(Assignee)

Comment 14

8 years ago
r63161. Filed bug 548340 to get the .dist-fennec's copied again (and hopefully set up to automatically copy in the future).
Depends on: 548340
(Assignee)

Updated

8 years ago
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 16

8 years ago
This _does_ work on Windows Mobile, leaving me stumped at the moment.
(Assignee)

Comment 17

8 years ago
Created attachment 428817 [details] [diff] [review]
*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.
Attachment #428817 - Flags: review?(laura)
Comment on attachment 428817 [details] [diff] [review]
*expletive deleted*

rofl.  but it works.
Attachment #428817 - Flags: review?(laura) → review+
(Assignee)

Comment 19

8 years ago
r63273. Reopening bug 548340.
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 21

8 years ago
Created attachment 428992 [details] [diff] [review]
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.
Attachment #428992 - Flags: review?(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).
Attachment #428992 - Flags: review?(laura) → review+
(Assignee)

Comment 23

8 years ago
r63484. Reopening the IT bug yet again.
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
Verified FIXED; tested on both the N900 and the N810 (and saw it working on WinMo).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.