Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workaround the non-portrait-orientation bug in our visibility routine. #180

Merged
merged 4 commits into from
Apr 22, 2014

Conversation

wearhere
Copy link
Contributor

While we work on #135, the workaround described on that bug should make it into 1.1. cc @MaxGabriel

@wearhere wearhere added this to the 1.1 milestone Apr 12, 2014
@MaxGabriel
Copy link
Contributor

Cool I can confirm this fixes the failing tests in the sample project that reproed the bug.

@wearhere wearhere changed the title Workaround the landscape-orientation bug in our visibility routine. Workaround the non-portrait-orientation bug in our visibility routine. Apr 12, 2014
@MaxGabriel
Copy link
Contributor

For the setOrientation timer thing, do you maybe want something that periodically checks the orientation returned from [[UIDevice currentDevice] orientation]? That way you could use a longer timeout and raise a clear exception if rotation failed. (Not sure why it'd be this unreliable though)

waitUntil(^BOOL{
        return [[UIDevice sharedDevice] orientation] == orientation;
    }, 4);
void waitUntil(BOOL (^waitBlock)(void), const NSTimeInterval timeout)
{
    NSCParameterAssert(waitBlock);
    NSCParameterAssert(timeout > 0);

    NSTimeInterval timeWaited = 0;
    while (true) {

        __block BOOL waitCondition = NO;
        dispatchSyncMainQueueIfNecessary(^{
            waitCondition = waitBlock();
        });

        if (waitCondition) {
            break;
        } else if (timeWaited >= timeout) {
            NSCAssert(@"Timeout exception", @"Hit timeout waiting for an event");
        } else {
            static const NSTimeInterval sleepInterval = 0.3;
            [NSThread sleepForTimeInterval:sleepInterval];
            timeWaited += sleepInterval;
        }
    }
}

void dispatchSyncMainQueueIfNecessary(void (^block)(void))
{
    NSCParameterAssert(block);
    if ([NSThread isMainThread]) {
        block();
    } else {
        dispatch_sync(dispatch_get_main_queue(), ^{
            block();
        });
    }
}

Jeff Wear added 3 commits April 20, 2014 01:17
…lve the elements.

The superclass' implementation looks like:

    [self waitUntilTappable:NO
          thenPerformActionWithUIARepresentation:^(NSString *UIARepresentation) {
        isVisible = [[[SLTerminal sharedTerminal] evalWithFormat:@"%@.isVisible()", UIARepresentation] boolValue];
    } timeout:0.0];

"Unknown objects" == objects of unknown classes like the accessibility elements
vended by `UIWebBrowserView`.

We shouldn't wait to resolve the element because `-isVisible` is supposed
to evaluate the current state.
…ntations. (refs #135)

As a workaround for the failure of our visibility routine.
…ate.

The previous delay was not long enough for the animation to complete on the iPad.
@wearhere
Copy link
Contributor Author

Oh, I think that the orientation does reliably update to that set--within the old time interval, actually, as the rotation test passes just fine. I just want to make sure that the rotation animation completes by the time that -setOrientation: returns, 'cause otherwise it'll look a bit weird from a user's standpoint and there might be problems with visibility/tappability.

I'm not sure why the tests failed the last time, but I don't think it was due to the orientation not being set right.

@wearhere
Copy link
Contributor Author

Found out why they failed before--on iOS 5.1 and 7.1, iPads forget their orientation when deactivated! The orientation actually becomes UIDeviceOrientationUnknown.

That's why this test run failed--when -[SLDeviceTest testDeactivateAppForDuration] ran before the visibility test, all the visibility test cases that test differences in behavior between Subliminal and UIAutomation started failing, because -[SLElement isVisible] thought that the device was in a non-portrait orientation and so used UIAElement.isVisible rather than Subliminal's visibility routine.

EDIT: Since this comment is linked from the commit history, I've clarified that it is iPads running iOS 5.1 or 7.1 that exhibit this issue.

wearhere pushed a commit that referenced this pull request Apr 20, 2014
On iOS 5.1, `UIDevice` forgets its orientation after deactivation.
This is not only unexpected but can mess with `-[SLElement isVisible]`
which (as of #180) falls back upon `UIAutomation`'s visibility routine
if (but only if) the device is in a non-portrait orientation.
@wearhere
Copy link
Contributor Author

Aaand UIDevice forgets on iOS 7.1 too >.<.

wearhere pushed a commit that referenced this pull request Apr 20, 2014
…refs #180)

On iOS 5.1 and 7.1, `UIDevice` forgets its orientation after deactivation.
This is not only unexpected but can mess with `-[SLElement isVisible]`
which (as of #180) falls back upon `UIAutomation`'s visibility routine
if (but only if) the device is in a non-portrait orientation.
@MaxGabriel
Copy link
Contributor

I wasn't able to replicate it, but maybe calling beginGeneratingDeviceOrientationNotifications before this test would fix it?

@wearhere
Copy link
Contributor Author

Further clarification: I see -[SLDeviceTest testThatDeviceOrientationPersistsThroughDeactivation] consistently fail on the iPad when running either iOS 5.1 or 7.1 (but 6.1's fine).

I don't doubt that it might work without the reset in some circumstances--in this run, some of the visibility test cases failed in a way that would suggest that the orientation had been forgotten, but not all, and -[SLDeviceTest testThatDeviceOrientationPersistsThroughDeactivation] passed there.

Calling -[UIDevice beginGeneratingDeviceOrientationNotifications] didn't seem to help fwiw.

…5.1 or 7.1. (refs #180)

On iPads running iOS 5.1 or 7.1, `UIDevice` forgets its orientation after deactivation.
This is not only unexpected but can mess with `-[SLElement isVisible]`
which (as of #180) falls back upon `UIAutomation`'s visibility routine
if (but only if) the device is in a non-portrait orientation.
ahaneyinkling added a commit that referenced this pull request Apr 22, 2014
Workaround the non-portrait-orientation bug in our visibility routine.
@ahaneyinkling ahaneyinkling merged commit afbf843 into master Apr 22, 2014
@ahaneyinkling ahaneyinkling deleted the jeff/135_landscape_visibility branch April 22, 2014 02:36
MaxGabriel added a commit to MaxGabriel/Subliminal that referenced this pull request Apr 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants