? Pending

User tests: Successful: Unsuccessful:

avatar csthomas
csthomas
19 Nov 2017

Pull Request for Issue #6859

Summary of Changes

Create a right cacheId (with suffix -M or -T) for mobile cache in redis and memcached based on memcache.
Add separate cache for tablet.
Check browser type (mobile, tablet, etc) only once on cache constructor.

Testing Instructions

See at #6859

avatar csthomas csthomas - open - 19 Nov 2017
avatar csthomas csthomas - change - 19 Nov 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 19 Nov 2017
Category Libraries
avatar csthomas csthomas - change - 20 Nov 2017
Labels Added: ?
avatar csthomas
csthomas - comment - 20 Nov 2017

I have fixed the code style, please take a look again.

Can someone restart the drone?

avatar csthomas
csthomas - comment - 23 Nov 2017

@hans2103 Do you thing that PR will be useful? Can you test it?

This PR does not use MobileDetect.net but it should be easy to use workaround:

  • reset JFactory::getConfig()->set('cache_platformprefix', 0); and manually set the cache option platformKey based on MobileDetect.net.
avatar csthomas
csthomas - comment - 6 Dec 2017

@ptest8794 @ggppdk
You may be interested in adding a comment or testing this PR.

avatar csthomas
csthomas - comment - 7 Dec 2017

@mbabker
Is there an chance to add composer require mobiledetect/mobiledetectlib to J3.x?
If yes then I would replace WebClient by Mobile_Detect in the next PR for J3.9.

avatar ptest8794
ptest8794 - comment - 8 Dec 2017

Hi

i've tested the changes on my test site.
with these mods the layout on tablet and mobile is loaded correctly.

But this changes not cover all tablets platforms (for example windows tablets)

If is it possible integrare mobile detect library in joomla core should be the better solution

avatar csthomas
csthomas - comment - 8 Dec 2017

I would like to add Mobile_Detect too but I do not know if we can do that in J3.x.

This PR is not perfect but is closer to our goal and after 2 tests has a chance to go with 3.8.x.

By using Mobile_Detect you will have to wait at least to J3.9 or J4.0.

avatar mbabker
mbabker - comment - 11 Dec 2017

If we're going to add Mobile_Detect we need a good plan for how to phase out the two other browser detection classes we have in the API and not just lump in a third.

avatar csthomas
csthomas - comment - 29 Dec 2017

I see that this PR did not satisfy anyone.
Without 2 successful test it won't be merged to joomla.

In the case of Mobile-Detect, this is a more complex issue, which was mentioned by @mbabker.

I'm not interested to extend this PR at now.
If there won't more comments/tests I will close this PR.

avatar csthomas csthomas - change - 31 Jan 2018
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2018-01-31 19:09:44
Closed_By csthomas
avatar csthomas csthomas - close - 31 Jan 2018

Add a Comment

Login with GitHub to post a comment