936433 - readingrockets.org sends desktop content to Firefox OS

Status: RESOLVED FIXED

(Web Compatibility :: Site Reports, defect)

Description

[Hallvord R. M. Steen [:hallvors]]

Site: http://www.readingrockets.org/
readingrockets.org sends desktop content to Firefox OS

:: Steps To Reproduce

Here's Firefox on Android mobile getting redirected:

GET / HTTP/1.1
User-Agent: Mozilla/5.0 (Android; Mobile; rv:18.0) Gecko/18.0 Firefox/18.0
Host: www.readingrockets.org

HTTP/1.1 302 Moved Temporarily
Server: Varnish
Location: http://m.readingrockets.org/
Accept-Ranges: bytes
Date: Fri, 08 Nov 2013 12:08:42 GMT
X-Varnish: 609723911
Age: 0
Via: 1.1 varnish
Connection: close
X-Cache-Svr: webnode2.readingrockets.org
X-Cache: MISS

Here's Firefox OS:

------------------------------------------------------------------
GET / HTTP/1.1
User-Agent: Mozilla/5.0 (Mobile; rv:18.0) Gecko/18.0 Firefox/18.0
Host: www.readingrockets.org

HTTP/1.1 200 OK
Server: Apache/2.2.14 (Ubuntu)
X-Powered-By: PHP/5.2.17-0ubuntu0ppa3~lucid
Set-Cookie: XARAYASID=9e84d6257082b9a6cf5165b7b9df2059; expires=Fri, 15-Nov-2013 11:14:01 GMT; path=/
Expires: Thu, 19 Nov 1981 08:52:00 GMT
Cache-Control: no-store, no-cache, must-revalidate, post-check=0, pre-check=0
Pragma: no-cache
Vary: Accept-Encoding
Content-Type: text/html; charset=iso-8859-1
Date: Fri, 08 Nov 2013 12:08:50 GMT
X-Varnish: 979495962 979476751
Age: 3286
Via: 1.1 varnish
Connection: keep-alive
X-Cache-Svr: webnode1.readingrockets.org
X-Cache: HIT
X-Cache-Hits: 98
Content-Length: 33030

------------------------------------------------------------------


:: Expected Result

mobile content

:: Actual Result

no mobile content

:: Additional Information

Software Version: 1.1
Reporter's User Agent: Mozilla/5.0 (Mobile; rv:18.0) Gecko/18.0 Firefox/18.0

Comment 1

[Adam Stevenson [:adamopenweb]]

Hallvord, the site is providing FFOS the mobile content now. But I can't get the site Menu to work on Firefox OS or Android. It works fine on Chrome. Can you test please?

Comment 2

[Mike Taylor [:miketaylr]]

Seems like they're using responsive design now. Confirmed that the menu doesn't work -- will take a closer look.

Comment 3

[Mike Taylor [:miketaylr]]

Logcat reports this when I tap on the menu:

E/GeckoPanZoomController(18274): Received impossible touch end while in WAITING_LISTENERS
E/GeckoPanZoomController(18274): Received impossible touch move while in WAITING_LISTENERS
E/GeckoPanZoomController(18274): Received impossible touch move while in WAITING_LISTENERS
E/GeckoPanZoomController(18274): Received impossible touch end while in WAITING_LISTENERS
E/GeckoPanZoomController(18274): Received impossible touch move while in WAITING_LISTENERS
E/GeckoPanZoomController(18274): Received impossible touch move while in WAITING_LISTENERS
E/GeckoPanZoomController(18274): Received impossible touch end while in WAITING_LISTENERS
E/GeckoPanZoomController(18274): Received impossible touch end while in WAITING_LISTENERS
E/GeckoPanZoomController(18274): Received impossible touch end while in WAITING_LISTENERSwhich 

There's a similar error message in https://bugzilla.mozilla.org/show_bug.cgi?id=800662

Comment 4

[Mike Taylor [:miketaylr]]

OK, those logcat errors aren't really the issue (though possibly a side effect).

The menu does work on http://www.readingrockets.org/shows/launching/fluent, and in the JS console of the homepage:

ReferenceError: g is not defined in the JS console when remote debugging, so that error might just be breaking the menu somehow.

Turns out this is a bug in a jQuery plugin they're using, having to do with function declaration hoisting in conditionals. It's been reported a few times at https://github.com/woothemes/FlexSlider/issues/958. Once you fix that issue, the menu works. 

I'll submit a patch and we can either ask them to manually apply the patch or update (if it gets accepted) to the new version.

Comment 5

[Mike Taylor [:miketaylr]]

Sent a patch: https://github.com/woothemes/FlexSlider/issues/958#ref-commit-8af7e0d

Comment 6

[Hallvord R. M. Steen [:hallvors]]

So Mike found it and fixed it, right? :-) Removing the needinfo.

Comment 7

[Adam Stevenson [:adamopenweb]]

Reached out using the sites contact form.

Comment 8

[Adam Stevenson [:adamopenweb]]

Acknowledged receipt of my email.

Comment 9

[Adam Stevenson [:adamopenweb]]

Great response. They are looking to implement the patch and really appreciate all the JS troubleshooting.

Comment 10

[Adam Stevenson [:adamopenweb]]

Issue with the menu has been fixed.

Comment 11

[Shalini Anand]

I see that you guys have already noticed that this was fixed :) So, all I wanted to add was this: your JS patch worked great (Thanks Mike!), except when minified, in which case I just had to add a semicolon after the new onTouchStart function declaration. I figured I'd mention it here, for context. It didn't seem appropriate to submit it as a separate patch to Woothemes, because it might just be a quirk of Drupal's minification process (we're using the Flexslider module on a Drupal site), and I didn't want to steal any of your glory. So, I thought you might just want to edit your submitted patch instead? Let me know what you prefer, or if you have any questions about our particular setup, etc.

Thanks again for your help!

Comment 12

[Mike Taylor [:miketaylr]]

Ah, cool Shalini. I realized I need to tweak that patch anyways... there's some other func decl's that I didn't even see in that conditional. >_<

Will update my PR and make sure semicolons are in there.

Comment 13

[Hallvord R. M. Steen [:hallvors]]

Mike, the patch will indeed make sure no "g is not defined" error is thrown, making sure the rest of the JS works as expected, but won't it break some of the actual *touch* functionality? The way the source code is organised after the patch is applied means you're still trying to add event listeners before the variable names actually reference a function? This won't work: http://jsfiddle.net/h8feL/

All the content I tried touching on readingrockets.org worked fine though. I guess the code may run multiple times so that the event listeners are added again later, when the variables are set - however it would be better to move all the addEventListener() calls to below all the function definitions.

Comment 14

[Hallvord R. M. Steen [:hallvors]]

(Of course, if the project is abandoned there may not be much point in spending time improving the patch :-/)

Comment 15

[Mike Taylor [:miketaylr]]

Whoops, good call Hallvord. Updated my PR.