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

bzimport set Reference to bz27238.
Peachey88 created this task.Via LegacyFeb 8 2011, 5:08 AM
Peachey88 added a comment.Via ConduitFeb 8 2011, 8:07 AM
  • Bug 27244 has been marked as a duplicate of this bug. ***
brion added a comment.Via ConduitFeb 8 2011, 8:20 AM

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.

brion added a comment.Via ConduitFeb 8 2011, 8:31 AM

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.

Reedy added a comment.Via ConduitFeb 8 2011, 9:52 AM

(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

brion added a comment.Via ConduitFeb 8 2011, 9:53 AM

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

bzimport added a comment.Via ConduitFeb 8 2011, 11:13 AM

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.

brion added a comment.Via ConduitFeb 8 2011, 11:20 AM

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.

brion added a comment.Via ConduitFeb 8 2011, 11:22 AM

Comment and reopen belonged on bug 27245.

Add Comment