Page MenuHomePhabricator

MobileFrontend doesn't remember user choice (mobile/desktop) in some use cases
Closed, ResolvedPublic

Description

With a tablet, each time I load a page I have to click on "desktop view" because the "mobile view" is always loaded, this independently of previous choices.

Conf:
$wgMFAutodetectMobileView = true;
$wgMobileFrontendFormatCookieExpiry = 4200000000;

MW 1.21.2 + last stable release of the extension.


Version: unspecified
Severity: trivial

Details

Reference
bz54885

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 2:34 AM
bzimport set Reference to bz54885.

bingle-admin wrote:

Prioritization and scheduling of this bug is tracked on Mingle card https://wikimedia.mingle.thoughtworks.com/projects/mobile/cards/1277

Do you have cookies enabled on that browser?

yes, cookie is activated on my Firefox (android edition)

Kelson, I'm unable to replicate the problem you describe. Can you provide some additional information:

  • When you say 'last stable release of the extension' what exactly do you mean? We do not package specific releases of the extension. Perhaps you can provide the sha1 of git checkout? (Note that current master of MobileFrontend will *not* work with Mediawiki 1.21.2)
  • What kind of table are you using? Or better yet, can you provide the User-Agent of your browser?
  • Is it possible for you to verify whether or not you get a cookie named 'stopMobileRedirect' set after tapping 'desktop view'?
  • Can you describe the architecture you're using to run MobileFrontend and in particular if you're using any kind of caching service like Squid or Varnish.

@Arthur
Thank you for your comment. Give me please a little bit time to lead this investigation and deliver the results here.

Marking as WORKSFORME until someone else can confirm the issue.

I have investigated a little bit and the problem is related to the "stopMobileRedirect" cookie.
The reaon is that the domain is wrong in with "exotic" setups (so the cookie does not apply).

For example if:

  • The server name has more than 3 parts (like wiki.wikimedia.org.uk) and you don't have setup $wgMFStopRedirectCookieHost.
  • You access the server under an other address like specified in $wgMFStopRedirectCookieHost

In a try to fix this I can propose following changes to MobileContext.php.
$ diff MobileContext.php.back MobileContext.php
473,477c473
< $domainParts = explode( '.', $host );
< $domainParts = array_reverse( $domainParts );
< // Although some browsers will accept cookies without the initial ., » RFC 2109 requires it to be included.
< wfProfileOut( METHOD );

< return count( $domainParts ) >= 2 ? '.' . $domainParts[1] . '.' . $domainParts[0] : $host;

return substr( $host, strpos( $host, "." ) );

493c489

< if ( !$wgMFStopRedirectCookieHost ) {

		if ( !$wgMFStopRedirectCookieHost || $wgMFStopRedirectCookieHost != $this->getRequest()->getHeader( 'Host' ) ) {

My previous patch does not fix the whole problem. If you have your wiki available at http://wikimedia.org.uk, the computed domain name will be ".org.uk". ".org.uk" is not accepted by Firefox (and probably other browsers) because this is a top level domain.

To fix this, I don't see any other solution than introducing a new configuration variable called $wgMFCookieDomain. Here would be the new patch.

$ diff MobileContext.php.back MobileContext.php
473,477c473
< $domainParts = explode( '.', $host );
< $domainParts = array_reverse( $domainParts );
< // Although some browsers will accept cookies without the initial ., » RFC 2109 requires it to be included.
< wfProfileOut( METHOD );

< return count( $domainParts ) >= 2 ? '.' . $domainParts[1] . '.' . $domainParts[0] : $host;

return substr( $host, strpos( $host, "." ) );

491c487,488

< global $wgMFStopRedirectCookieHost;

		global $wgMFStopRedirectCookieHost, $wgMFCookieDomain;
		$host = $this->getRequest()->getHeader( 'Host' );

493,494c490,495
< if ( !$wgMFStopRedirectCookieHost ) {

< $wgMFStopRedirectCookieHost = $this->getBaseDomain();

		if ( !$wgMFStopRedirectCookieHost || $wgMFStopRedirectCookieHost != $host ) {
		        if ( $wgMFCookieDomain && ( strpos( $host, $wgMFCookieDomain ) !== false )) {
			  $wgMFStopRedirectCookieHost = $wgMFCookieDomain;
			} else {
			  $wgMFStopRedirectCookieHost = $this->getBaseDomain();
			}

Kelson, thanks for digging deeper and following up. Will you submit your patch to gerrit?

Kelson, just wanted to poke again - can you please submit your patch to gerrit? It will be easier to review, test, and make comments on it there.

If the patch was added as an attachment here one could use Valhallsw's script to upload it via http://tools.wmflabs.org/gerrit-patch-uploader/ ...

Will do it, I don't have forgotten...

Change 117699 had a related patch set uploaded by Jdlrobson:
Remember user choice (mobile/desktop)

https://gerrit.wikimedia.org/r/117699

Change 117699 abandoned by Jdlrobson:
Remember user choice (mobile/desktop)

Reason:
Abandoning to clear the code review queue due to lack of activity. Please re-submit it when you have the time. Thanks!

https://gerrit.wikimedia.org/r/117699

also, when on https, the stopMobileRedirect cookie is set with the 'secure' flag on. That means it won't be visible if the user enters from an http:// url from google for instance...

Is this intentional ?

What is the status of this bug now?

bingle-admin wrote:

Prioritization and scheduling of this bug is tracked on Trello card https://trello.com/c/gtF11D1B

Thank you Jon for your patience. I just failed to give enough time to provide a correct patch here (I'm not so familiar with the whole testing infrastructure).

It works for the WMUK instance and this is the most important to me.

But it still on my TODO list... or someone else will do the patch as the bug/patch relatively trivial is.

(In reply to Derk-Jan Hartman from comment #17)

also, when on https, the stopMobileRedirect cookie is set with the 'secure'
flag on. That means it won't be visible if the user enters from an http://
url from google for instance...

Is this intentional ?

This was fixed as part of bug 68347.

Kelson,

This bug has been added to the Mobile Web team's current sprint (see https://trello.com/c/gtF11D1B) and I'm taking a look.

I've read through the patch that Jon submitted on your behalf and it looks okay. However, I'm wondering what you had/have the wgMFStopMobileRedirect configuration variable set to. I ask because that variable seems to be used to override MobileContext#getBaseDomain in #getStopMobileRedirectCookieDomain.

Sorry, wgMFStopRedirectCookieHost.

@sam

Thank you for trying to push this bug/patch. I have explained in
https://bugzilla.wikimedia.org/show_bug.cgi?id=54885#c8 why I had to create this new configuration value.

No problem Kelson.

MobileContext#getStopMobileRedirectCookieDomain returns wgMFStopRedirectCookieHost when it's defined. So why not set wgMFStopRedirectCookieHost to '.wikimedia.org.uk', rather than create another mechanism for setting the cookie domain?

Marking as unconfirmed until we hear back from Kelson.

I can only assume this problem has gone away due to the lack of discussion?