lyspring / adwhirl

Automatically exported from code.google.com/p/adwhirl
0 stars 0 forks source link

iAds can serve landscape ads in a portrait-only view controller #113

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Create a new, portrait only view controller, with an AdWhirlView using the 
iAd network.
2. Wait for the first ad to load, then rotate the device to landscape 
orientation and wait for the next ad to appear.

What is the expected output? What do you see instead?
I expect to see portrait-style ads, but instead a landscape-style ad appears.

What version of the product are you using? On what operating system?
iOS SDK 2.5.5, though examination of the source code shows this problem likely 
exists in 2.6.0.

Please provide any additional information below.
AdWhirlAdapterIAd should query through to the view controller to determine 
valid interface orientations.

The main problem is this code in -[AdWhirlAdapterIAd getAd]:

  UIDeviceOrientation orientation;
  if ([self.adWhirlDelegate respondsToSelector:@selector(adWhirlCurrentOrientation)]) {
    orientation = [self.adWhirlDelegate adWhirlCurrentOrientation];
  }
  else {
    orientation = [UIDevice currentDevice].orientation;
  }

The documentation states that -adWhirlCurrentOrientation only must be 
implemented when the user interface's notion of orientation differs from 
[UIDevice currentDevice].orientation, but this is not strictly true. [UIDevice 
currentDevice].orientation returns the orientation of the device without 
considering the support orientations of the current view controller, and will 
lead to the behavior described by this bug: while the rest of the UI remains in 
portrait, the ad banner, still sized and positioned in a portrait style, 
displays a landscape ad.

A better method of testing for this case is to first check if the 
AdWhirlDelegate is a subclass of UIViewController, and query -[UIViewController 
interfaceOrientation]. If that fails, falling back to adWhirlCurrentOrientation 
or [UIDevice currentDevice].orientation is a reasonable thing to do.

Note that a workaround for now is to simple implement 
adWhirlCurrentOrientation, returning self.interfaceOrientation.

A secondary issue is that the iAd adapter always claims support for both 
portrait and landscape banners without consulting the delegate. Again, if the 
delegate is a UIViewController, querying 
shouldAutorotateToInterfaceOrientation: would be better, avoiding the download 
of ad banner formats that should never be shown.

Original issue reported on code.google.com by steve.ma...@gmail.com on 1 Oct 2010 at 3:03

GoogleCodeExporter commented 8 years ago
The AdWhirl implementation using [UIDevice currentDevice] does not consider 
when the device is face up or down. It should contain something like this-

    if( UIDeviceOrientationIsValidInterfaceOrientation(interfaceOrientation) )
        interfaceOrientation = UIDeviceOrientationPortrait;

To make it one of the 4 valid vertical positions (2 portrait/2 landscape).

Even better is to cache the last valid orientation and use it if the device is 
laying down. You should implement-

- (UIDeviceOrientation)adWhirlCurrentOrientation

in your code and set a valid orientation using the above logic

Original comment by Geb...@gmail.com on 1 Oct 2010 at 6:34

GoogleCodeExporter commented 8 years ago

Original comment by nigelc...@google.com on 7 Oct 2010 at 12:34

GoogleCodeExporter commented 8 years ago

Original comment by fch...@google.com on 20 Oct 2010 at 12:14