Page MenuHomePhabricator

Regression: change to the redirection of the iPad users
Closed, ResolvedPublic

Details

Reference
bz27238

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 11:18 PM
bzimport set Reference to bz27238.
  • Bug 27244 has been marked as a duplicate of this bug. ***

Culprit is r81128 (was merged to 1.16wmf4 in r67681).

It hasn't been merged to 1.17wmf1, so this behavior may disappear from production shortly.

Still needs to be fixed on trunk.

I've reverted the change in r81700; the test cases were mysteriously deleted previously, so that's all of r81128 now reverted on trunk.

It looks like it was *intended* to fix Android tablets to not redirect to the mobile site, but apparently did so by removing any checks for Android, and replacing the Android, iPhone, and iPod checks with a "Mobile Safari" check which of course slapped the iPad across the head, exactly what shouldn't be done for a tablet.

A proper fix for that other problem would still need to be made.

(In reply to comment #3)

I've reverted the change in r81700; the test cases were mysteriously deleted
previously, so that's all of r81128 now reverted on trunk.

It looks like it was *intended* to fix Android tablets to not redirect to the
mobile site, but apparently did so by removing any checks for Android, and
replacing the Android, iPhone, and iPod checks with a "Mobile Safari" check
which of course slapped the iPad across the head, exactly what shouldn't be
done for a tablet.

A proper fix for that other problem would still need to be made.

Test cases were deleted by hampton as an easier way for tomasz to commit it live (not having to add a directory).

However, we didn't work out why the hell he didn't just svn move them up a level into the main directory

Broke the android issue out to bug 27245 for a fresh fix.

hcatlin wrote:

Ok, so lets go through these. The test cases were not supposed to be deleted. I *thought* I moved them. But, since I haven't used SVN in any capacity in 10 years, I f*cked it up.

Sorry!

Should be closed in r81714 with more tests and a tweaked UA regex.

See my comments on code review for r81714; this regex is super unclear as to its purpose and is likely very fragile, and the test cases don't appear to include any cases for the Android 3.0 tablets that the change is meant to address.

Comment and reopen belonged on bug 27245.